From 1635811c3dd9ca18aba661e85d9c3a6617ad0458 Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Tue, 19 Aug 2025 11:33:44 -0700 Subject: [PATCH 01/10] add checklist in pr template Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .github/pull_request_template.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 4665a9682a3..64b7367d7bb 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -40,6 +40,16 @@ Please explain the issue and the solution in short. Please list clearly what are the relevant test(s) that can safeguard the changes in the PR. This helps us to ensure we have sufficient test coverage for the PR. --> +## PR Checklist +Please check the following items wherever appropriate, else strick them out if N/A. This is needed to merge the PR. +All items must be resolved either by checking boxes or striking the line if N/A. This will be needed to merge the PR. +- [ ] Have you made sure the PR description is explaining what you’re doing and why? +- [ ] Are you following the [TRT-LLM CODING GUIDELINES](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md) to the best of your knowledge? +- [ ] Do you have test cases for new code paths? See https://github.com/NVIDIA/TensorRT-LLM/tree/main/tests#1-how-does-the-ci-work for instructions on how to add. +- [ ] Have you introduced any new dependencies? If so, please make sure that scans have been run for license permissiveness and vulnerability issues recursively for the dependency and its dependencies. +- [ ] Have you updated https://github.com/NVIDIA/TensorRT-LLM/blob/main/.github/CODEOWNERS to reflect any changes in ownership due to your PR? +- [ ] Have you updated any documentation that is associated with your change? + ## GitHub Bot Help `/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...` From b833c3b7b17aea73d05653dc1afc9441c94e9117 Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Tue, 19 Aug 2025 11:34:15 -0700 Subject: [PATCH 02/10] include action Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .github/workflows/pr-check.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 0668283cc30..7fd775773b1 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -53,3 +53,10 @@ jobs: echo " - [#1234][doc] Update documentation" echo " - [None][chore] Minor clean-up" exit 1 + + checklist: + name: Validate PR Checklist + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: venkywonka/goggles_action/actions/pr_checklist@pr-checklist From 2944a1e11704abeb09fe4762680ea1104c4fbef0 Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Tue, 19 Aug 2025 15:05:54 -0700 Subject: [PATCH 03/10] use goggles_action for pr checklist enforcement Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .github/workflows/pr-checklist.yml | 26 +++++++++++++++++++ .../{pr-check.yml => pr-title-check.yml} | 7 ----- 2 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/pr-checklist.yml rename .github/workflows/{pr-check.yml => pr-title-check.yml} (91%) diff --git a/.github/workflows/pr-checklist.yml b/.github/workflows/pr-checklist.yml new file mode 100644 index 00000000000..da488e4bc64 --- /dev/null +++ b/.github/workflows/pr-checklist.yml @@ -0,0 +1,26 @@ +name: Validate PR Checklist + +on: + pull_request: + types: [opened, edited, synchronize] + +permissions: + contents: read + pull-requests: read + +jobs: + pr-checklist: + runs-on: ubuntu-latest + steps: + - name: Checkout private action repository + uses: actions/checkout@v4 + with: + repository: poweiw/goggles_action + path: ./.github/actions/goggles_action + token: ${{ secrets.GOGGLES_ACTION_REPO_TOKEN}} + ref: test/pr-checklist-20250819 + + - name: Validate PR Checklist + uses: ./.github/actions/goggles_action/actions/pr_checklist + with: + PR_BODY: ${{ github.event.pull_request.body }} diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-title-check.yml similarity index 91% rename from .github/workflows/pr-check.yml rename to .github/workflows/pr-title-check.yml index 7fd775773b1..0668283cc30 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-title-check.yml @@ -53,10 +53,3 @@ jobs: echo " - [#1234][doc] Update documentation" echo " - [None][chore] Minor clean-up" exit 1 - - checklist: - name: Validate PR Checklist - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: venkywonka/goggles_action/actions/pr_checklist@pr-checklist From 1c4fd92f3d98144e2df18b7fa05b60ebae0dd5b4 Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Tue, 19 Aug 2025 15:28:59 -0700 Subject: [PATCH 04/10] run on pull_request_target Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .github/workflows/pr-checklist.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-checklist.yml b/.github/workflows/pr-checklist.yml index da488e4bc64..a38dabe54b3 100644 --- a/.github/workflows/pr-checklist.yml +++ b/.github/workflows/pr-checklist.yml @@ -1,7 +1,7 @@ name: Validate PR Checklist on: - pull_request: + pull_request_target: types: [opened, edited, synchronize] permissions: From 30f0a2117c3748ef5b6b8890696de6f21b64ad58 Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Tue, 19 Aug 2025 16:59:16 -0700 Subject: [PATCH 05/10] enforce existence of a checklist in pr Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .github/workflows/pr-checklist.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/pr-checklist.yml b/.github/workflows/pr-checklist.yml index a38dabe54b3..a3518481847 100644 --- a/.github/workflows/pr-checklist.yml +++ b/.github/workflows/pr-checklist.yml @@ -24,3 +24,4 @@ jobs: uses: ./.github/actions/goggles_action/actions/pr_checklist with: PR_BODY: ${{ github.event.pull_request.body }} + ENFORCE_PR_HAS_CHECKLIST: true From 4fce2fdf2cb359297b8de79dd2656f10e5c496c8 Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:28:51 -0700 Subject: [PATCH 06/10] reduce checking overhead Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .github/pull_request_template.md | 20 ++++++++++++-------- .github/workflows/pr-checklist.yml | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 64b7367d7bb..1fa777b22e9 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -41,14 +41,18 @@ Please list clearly what are the relevant test(s) that can safeguard the changes --> ## PR Checklist -Please check the following items wherever appropriate, else strick them out if N/A. This is needed to merge the PR. -All items must be resolved either by checking boxes or striking the line if N/A. This will be needed to merge the PR. -- [ ] Have you made sure the PR description is explaining what you’re doing and why? -- [ ] Are you following the [TRT-LLM CODING GUIDELINES](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md) to the best of your knowledge? -- [ ] Do you have test cases for new code paths? See https://github.com/NVIDIA/TensorRT-LLM/tree/main/tests#1-how-does-the-ci-work for instructions on how to add. -- [ ] Have you introduced any new dependencies? If so, please make sure that scans have been run for license permissiveness and vulnerability issues recursively for the dependency and its dependencies. -- [ ] Have you updated https://github.com/NVIDIA/TensorRT-LLM/blob/main/.github/CODEOWNERS to reflect any changes in ownership due to your PR? -- [ ] Have you updated any documentation that is associated with your change? + +Please review the following before submitting your PR: +- PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense. +- PR Follows [TRT-LLM CODING GUIDELINES](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md) to the best of your knowledge. +- Test cases are provided for new code paths (see [test instructions](https://github.com/NVIDIA/TensorRT-LLM/tree/main/tests#1-how-does-the-ci-work)) +- Any new dependencies have been scanned for license and vulnerabilities +- [CODEOWNERS](https://github.com/NVIDIA/TensorRT-LLM/blob/main/.github/CODEOWNERS) updated if ownership changes +- Documentation updated as needed +- The reviewers assigned automatically/manually are appropriate for the PR. + + +- [ ] Please check this after reviewing the above items as appropriate for this PR. ## GitHub Bot Help diff --git a/.github/workflows/pr-checklist.yml b/.github/workflows/pr-checklist.yml index a3518481847..dfec07e5400 100644 --- a/.github/workflows/pr-checklist.yml +++ b/.github/workflows/pr-checklist.yml @@ -18,7 +18,7 @@ jobs: repository: poweiw/goggles_action path: ./.github/actions/goggles_action token: ${{ secrets.GOGGLES_ACTION_REPO_TOKEN}} - ref: test/pr-checklist-20250819 + ref: 0dc6365 - name: Validate PR Checklist uses: ./.github/actions/goggles_action/actions/pr_checklist From 0f29718586d575fb142e832ff2a812ddfffa28a3 Mon Sep 17 00:00:00 2001 From: Venky <23023424+venkywonka@users.noreply.github.com> Date: Tue, 26 Aug 2025 09:42:24 -0700 Subject: [PATCH 07/10] use `on:pull_request` instead of `on:pull_request_target` Signed-off-by: Venky <23023424+venkywonka@users.noreply.github.com> --- .github/workflows/pr-checklist.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-checklist.yml b/.github/workflows/pr-checklist.yml index dfec07e5400..3c0f887cdfd 100644 --- a/.github/workflows/pr-checklist.yml +++ b/.github/workflows/pr-checklist.yml @@ -1,7 +1,7 @@ name: Validate PR Checklist on: - pull_request_target: + pull_request: types: [opened, edited, synchronize] permissions: From d21348c71de6007c4e1679491179a35556edb69f Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Wed, 27 Aug 2025 10:48:49 -0700 Subject: [PATCH 08/10] Refactor PR checklist workflow and add validation script - Introduced a new script to validate the presence and status of checklist items in PR descriptions, enforcing checklist requirements. - Now instead of borrowing the action from goggles_action, we maintain it in TRTLLM itself. Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .github/workflows/pr-checklist.yml | 16 ++--- scripts/pr_checklist_check.py | 104 +++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 8 deletions(-) create mode 100644 scripts/pr_checklist_check.py diff --git a/.github/workflows/pr-checklist.yml b/.github/workflows/pr-checklist.yml index 3c0f887cdfd..26153f44879 100644 --- a/.github/workflows/pr-checklist.yml +++ b/.github/workflows/pr-checklist.yml @@ -12,16 +12,16 @@ jobs: pr-checklist: runs-on: ubuntu-latest steps: - - name: Checkout private action repository + - name: Checkout repository uses: actions/checkout@v4 - with: - repository: poweiw/goggles_action - path: ./.github/actions/goggles_action - token: ${{ secrets.GOGGLES_ACTION_REPO_TOKEN}} - ref: 0dc6365 - - name: Validate PR Checklist - uses: ./.github/actions/goggles_action/actions/pr_checklist + - name: Set up Python + uses: actions/setup-python@v5 with: + python-version: '3.10' + + - name: Run validation script + env: PR_BODY: ${{ github.event.pull_request.body }} ENFORCE_PR_HAS_CHECKLIST: true + run: python scripts/pr_checklist_check.py diff --git a/scripts/pr_checklist_check.py b/scripts/pr_checklist_check.py new file mode 100644 index 00000000000..9cff4cdccd7 --- /dev/null +++ b/scripts/pr_checklist_check.py @@ -0,0 +1,104 @@ +import os +import re +import sys +from typing import List + +# Matches a Markdown checklist item in the PR body. +# Expected format: "- [ ] Task description" or "* [x] Task description" +# Group 1 captures the checkbox state: ' ' (unchecked), 'x' or 'X' (checked). +# Group 2 captures the task content (the description of the checklist item). +TASK_PATTERN = re.compile(r'^\s*[-*]\s+\[( |x|X)\]\s*(.*)') + + +def find_all_tasks(pr_body: str) -> List[str]: + """Return list of all task list items (both resolved and unresolved).""" + tasks: List[str] = [] + for line in pr_body.splitlines(): + match = TASK_PATTERN.match(line) + if match: + tasks.append(match.group(0).strip()) + return tasks + + +def find_unresolved_tasks(pr_body: str) -> List[str]: + """Return list of unresolved task list items. + + A task is considered resolved if it is checked (``[x]`` or ``[X]``) + or if its text is struck through using ``~~`` markers. + """ + unresolved: List[str] = [] + for line in pr_body.splitlines(): + match = TASK_PATTERN.match(line) + if not match: + continue + state, content = match.groups() + if state.lower() == 'x': + continue + # Check if the entire content is struck through + if content.strip().startswith('~~') and content.strip().endswith('~~'): + continue + unresolved.append(match.group(0).strip()) + return unresolved + + +def check_pr_checklist_section(pr_body: str) -> tuple[bool, str]: + """Check if the PR Checklist section exists with the required final checkbox. + + Returns: + tuple: (is_valid, error_message) + """ + # Check if "## PR Checklist" header exists + pr_checklist_pattern = re.compile(r'^##\s+PR\s+Checklist', + re.IGNORECASE | re.MULTILINE) + if not pr_checklist_pattern.search(pr_body): + return False, "Missing '## PR Checklist' header. Please ensure you haven't removed the PR template section." + + # Check if the final checkbox exists (the one users must check) + final_checkbox_pattern = re.compile( + r'^\s*[-*]\s+\[( |x|X)\]\s+Please check this after reviewing the above items', + re.MULTILINE) + if not final_checkbox_pattern.search(pr_body): + return False, "Missing the required final checkbox '- [ ] Please check this after reviewing the above items as appropriate for this PR.' Please ensure you haven't removed this from the PR template." + + return True, "" + + +def main() -> None: + pr_body = os.environ.get("PR_BODY", "") + enforce_checklist = os.environ.get("ENFORCE_PR_HAS_CHECKLIST", + "false").lower() == "true" + + # Always check for PR Checklist section when enforcement is enabled + if enforce_checklist: + is_valid, error_msg = check_pr_checklist_section(pr_body) + if not is_valid: + print(f"Error: {error_msg}") + sys.exit(1) + + all_tasks = find_all_tasks(pr_body) + unresolved = find_unresolved_tasks(pr_body) + + # Check if we need to enforce the presence of at least one checklist item + if enforce_checklist and not all_tasks: + print( + "Error: PR body must contain at least one checklist item when ENFORCE_PR_HAS_CHECKLIST is enabled." + ) + print( + "Expected format: - [ ] Task description or * [ ] Task description") + sys.exit(1) + + # If we have tasks, check if any are unresolved + if unresolved: + print("Unresolved checklist items found:") + for item in unresolved: + print(f"{item}") + sys.exit(1) + + if all_tasks: + print("All checklist items resolved.") + else: + print("No checklist items found in PR body.") + + +if __name__ == "__main__": + main() From fee1221ce79ad51aba5dca74218254da7e51038b Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Wed, 27 Aug 2025 10:56:52 -0700 Subject: [PATCH 09/10] consolidate. Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .../{pr-title-check.yml => pr-check.yml} | 18 +++++++++++++ .github/workflows/pr-checklist.yml | 27 ------------------- 2 files changed, 18 insertions(+), 27 deletions(-) rename .github/workflows/{pr-title-check.yml => pr-check.yml} (81%) delete mode 100644 .github/workflows/pr-checklist.yml diff --git a/.github/workflows/pr-title-check.yml b/.github/workflows/pr-check.yml similarity index 81% rename from .github/workflows/pr-title-check.yml rename to .github/workflows/pr-check.yml index 0668283cc30..1312b846e9b 100644 --- a/.github/workflows/pr-title-check.yml +++ b/.github/workflows/pr-check.yml @@ -53,3 +53,21 @@ jobs: echo " - [#1234][doc] Update documentation" echo " - [None][chore] Minor clean-up" exit 1 + + check-pr-body-checklist: + name: Check PR Checklist Resolution + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.10' + + - name: Validate PR Checklist + env: + PR_BODY: ${{ github.event.pull_request.body }} + ENFORCE_PR_HAS_CHECKLIST: true + run: python scripts/pr_checklist_check.py diff --git a/.github/workflows/pr-checklist.yml b/.github/workflows/pr-checklist.yml deleted file mode 100644 index 26153f44879..00000000000 --- a/.github/workflows/pr-checklist.yml +++ /dev/null @@ -1,27 +0,0 @@ -name: Validate PR Checklist - -on: - pull_request: - types: [opened, edited, synchronize] - -permissions: - contents: read - pull-requests: read - -jobs: - pr-checklist: - runs-on: ubuntu-latest - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - - name: Set up Python - uses: actions/setup-python@v5 - with: - python-version: '3.10' - - - name: Run validation script - env: - PR_BODY: ${{ github.event.pull_request.body }} - ENFORCE_PR_HAS_CHECKLIST: true - run: python scripts/pr_checklist_check.py From dc1f9a3bfcb792fbee16bdff981b8ebcdbaf0aac Mon Sep 17 00:00:00 2001 From: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> Date: Wed, 27 Aug 2025 11:02:04 -0700 Subject: [PATCH 10/10] add copyright, relocate script Signed-off-by: Venky Ganesh <23023424+venkywonka@users.noreply.github.com> --- .../scripts}/pr_checklist_check.py | 16 ++++++++++++++++ .github/workflows/pr-check.yml | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) rename {scripts => .github/scripts}/pr_checklist_check.py (84%) diff --git a/scripts/pr_checklist_check.py b/.github/scripts/pr_checklist_check.py similarity index 84% rename from scripts/pr_checklist_check.py rename to .github/scripts/pr_checklist_check.py index 9cff4cdccd7..e0c40e6c973 100644 --- a/scripts/pr_checklist_check.py +++ b/.github/scripts/pr_checklist_check.py @@ -1,3 +1,19 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import os import re import sys diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 1312b846e9b..787e5ac0132 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -70,4 +70,4 @@ jobs: env: PR_BODY: ${{ github.event.pull_request.body }} ENFORCE_PR_HAS_CHECKLIST: true - run: python scripts/pr_checklist_check.py + run: python .github/scripts/pr_checklist_check.py