[None] infra: add PR template checklist and action to require it #6959
Conversation
Signed-off-by: Venky Ganesh <[email protected]>
📝 WalkthroughWalkthroughReplaces the PR template Description section with a new PR Checklist and retains existing sections. Adds a GitHub Actions job to require resolved checklist items in PR descriptions, using mheap/require-checklist-action with continue-on-error enabled. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub PR
participant WF as Workflow: pr-check.yml
participant AC as Action: mheap/require-checklist-action@v2
Dev->>GH: Open/Update PR (description with checklist)
GH-->>WF: Trigger require-pr-checklist job
WF->>AC: Run checklist validation (requireChecklist: true)
AC-->>WF: Report success/failure (non-blocking due to continue-on-error)
WF-->>GH: Post check result on PR
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Signed-off-by: Venky Ganesh <[email protected]>
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/pull_request_template.md (1)
29-36: Fix typo/grammar and convert bare URLs to Markdown links for lint compliance
- Typo: “strick” → “strike”; “wherever” → “where.”
- Style: “make sure” → “ensure.”
- Replace bare URLs to satisfy markdownlint (MD034) and improve readability.
## PR Checklist -Please check the following items wherever appropriate, else strick them out if N/A. This is needed to merge the PR. +Please check the following items where appropriate, or strike them out if N/A. Completing this checklist is required to merge this 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? +- [ ] Do you have test cases for new code paths? See the [tests README: “How does the CI work?”](https://github.com/NVIDIA/TensorRT-LLM/tree/main/tests#1-how-does-the-ci-work) for instructions on how to add tests. +- [ ] Have you introduced any new dependencies? If so, please ensure that license and vulnerability scans have been run recursively on the dependency and its transitive dependencies. +- [ ] Have you updated [CODEOWNERS](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/workflows/pr-checklist.yml (1)
9-15: Tighten security and reliability: pin action by SHA and set minimal permissions
- Pin actions to a full commit SHA to mitigate supply-chain risk.
- Restrict default token permissions to least privilege for read-only checks.
-name: Require PR Checklist Resolution +name: Require PR Checklist Resolution +permissions: + contents: read + pull-requests: read + jobs: job1: runs-on: ubuntu-latest steps: - - uses: mheap/require-checklist-action@v2 + - uses: mheap/require-checklist-action@<PINNED_COMMIT_SHA> with: requireChecklist: trueIf you want, I can look up the current latest v2 commit SHA and update this for you.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/pull_request_template.md(1 hunks).github/workflows/pr-checklist.yml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.github/pull_request_template.md
[grammar] ~29-~29: There might be a mistake here.
Context: ...overage for the PR. --> ## PR Checklist Please check the following items whereve...
(QB_NEW_EN)
[style] ~34-~34: Consider using a different verb to strengthen your wording.
Context: ...ced any new dependencies? If so, please make sure that scans have been run for license pe...
(MAKE_SURE_ENSURE)
🪛 markdownlint-cli2 (0.17.2)
.github/pull_request_template.md
33-33: Bare URL used
(MD034, no-bare-urls)
35-35: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (1)
.github/workflows/pr-checklist.yml (1)
13-15: No additional flag needed for completeness enforcementThe mheap/require-checklist-action@v2 already fails the workflow when any checklist item is unchecked. The
requireChecklist: trueinput only ensures that at least one checklist is present (it errors if none are found); you don’t need a separate flag to block merges on incomplete items.• Ensure this step runs on your pull_request (or issue) trigger so incomplete items cause failure
• If you need to exempt certain checklist lines, useskipDescriptionRegex/skipDescriptionRegexFlagsto mark them optional
.github/workflows/pr-checklist.yml
Outdated
| on: | ||
| pull_request: | ||
| types: [opened, edited, synchronize] | ||
| issues: | ||
| types: [opened, edited, deleted] | ||
|
|
There was a problem hiding this comment.
Do not run this workflow on “issues” events; it will fail or spam checks on normal issues
Triggering on issues causes this job to run for every issue opened/edited/deleted, potentially failing when issue bodies don’t contain PR checklists and adding noise to issue workflows.
Consider removing the issues trigger (preferred) or guarding the job to run only for pull requests:
Option A — remove issues trigger:
on:
pull_request:
types: [opened, edited, synchronize]
- issues:
- types: [opened, edited, deleted]Option B — keep triggers but guard the job:
jobs:
job1:
+ if: github.event_name == 'pull_request'
runs-on: ubuntu-latest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| pull_request: | |
| types: [opened, edited, synchronize] | |
| issues: | |
| types: [opened, edited, deleted] | |
| on: | |
| pull_request: | |
| types: [opened, edited, synchronize] |
🤖 Prompt for AI Agents
.github/workflows/pr-checklist.yml lines 3-8: the workflow is mistakenly
configured to trigger on issues events which will run the job for every issue
and spam/fail checks; either remove the entire "issues" trigger block so the
workflow only runs on pull_request events (preferred), or keep it but add a
guard at the job level so it only executes for pull requests (for example check
github.event_name == 'pull_request' or inspect github.event.action to ensure
this runs only for PR contexts).
Signed-off-by: Venky Ganesh <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/pr-check.yml (2)
64-64: Pin third‑party GitHub Actions to immutable commit SHAs.For supply-chain safety and reproducibility, pin actions to a commit SHA instead of a mutable tag. Do this for both:
- mheap/require-checklist-action@v2
- agenthunt/[email protected]
Example:
- uses: mheap/require-checklist-action@
- uses: agenthunt/conventional-commit-checker-action@
Would you like me to fetch the latest SHAs and propose exact replacements?
16-21: Restrict workflow permissions to least privilege.Explicitly scoping permissions helps harden CI. This workflow only needs read access.
Apply this diff:
name: PR Checks -on: +permissions: + contents: read + pull-requests: read + +on: pull_request: types: [opened, edited, synchronize, reopened]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/pr-check.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (1)
.github/workflows/pr-check.yml (1)
62-67: requireChecklist semantics confirmed
requireChecklist: trueenforces both “fail on empty body” (no checklists) and “fail on any incomplete items” (all unchecked boxes).- There is no separate “require all checked” flag—unchecked items always trigger a failure by default.
- If you only want to enforce completion when a checklist exists (but not fail on empty descriptions), set
requireChecklist: false.No additional inputs are needed here.
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check if PR Description has checkboxes resolved | ||
| id: check-pr-descr-all-checked | ||
| uses: mheap/require-checklist-action@v2 | ||
| continue-on-error: true | ||
| with: | ||
| requireChecklist: true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Checklist enforcement currently non-blocking due to continue-on-error; job won’t fail when unchecked.
continue-on-error: true makes the step non-fatal, and there’s no follow-up step to flip the job to failure. As a result, unchecked boxes won’t prevent merging—undercutting the goal of “Require PR Checklist Resolution.”
Mirror the pattern you already use in the PR title job: keep continue-on-error: true for a friendly message, then add a gated step that exits 1 on failure.
Apply this diff:
- name: Check if PR Description has checkboxes resolved
id: check-pr-descr-all-checked
uses: mheap/require-checklist-action@v2
continue-on-error: true
with:
requireChecklist: true
+ - name: PR Checklist Guide
+ if: steps.check-pr-descr-all-checked.outcome == 'failure'
+ run: |
+ echo "::error::PR checklist validation failed."
+ echo ""
+ echo "Please ensure your PR description contains the checklist from"
+ echo ".github/pull_request_template.md and that all items are checked."
+ echo ""
+ echo "Tip: Use GitHub-style checkboxes:"
+ echo "- [x] Item completed"
+ echo "- [ ] Item not completed"
+ exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Check if PR Description has checkboxes resolved | |
| id: check-pr-descr-all-checked | |
| uses: mheap/require-checklist-action@v2 | |
| continue-on-error: true | |
| with: | |
| requireChecklist: true | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Check if PR Description has checkboxes resolved | |
| id: check-pr-descr-all-checked | |
| uses: mheap/require-checklist-action@v2 | |
| continue-on-error: true | |
| with: | |
| requireChecklist: true | |
| - name: PR Checklist Guide | |
| if: steps.check-pr-descr-all-checked.outcome == 'failure' | |
| run: | | |
| echo "::error::PR checklist validation failed." | |
| echo "" | |
| echo "Please ensure your PR description contains the checklist from" | |
| echo ".github/pull_request_template.md and that all items are checked." | |
| echo "" | |
| echo "Tip: Use GitHub-style checkboxes:" | |
| echo "- [x] Item completed" | |
| echo "- [ ] Item not completed" | |
| exit 1 |
🤖 Prompt for AI Agents
In .github/workflows/pr-check.yml around lines 60-67, the checklist step is
non-fatal because it uses "continue-on-error: true" with no follow-up; add a
subsequent gated step (keeping the existing friendly step unchanged) that checks
the checklist step's outcome (e.g. if: steps.check-pr-descr-all-checked.outcome
!= 'success') and explicitly fails the job (run an exit 1) when the checklist
did not pass, mirroring the PR title job pattern so the step provides a friendly
message but the workflow ultimately fails when boxes are unchecked.
|
closign this in favour of #6029 |
1 participant
Summary by CodeRabbit
Documentation
Chores
Description
Test Coverage
GitHub Bot Help
/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...Provide a user friendly way for developers to interact with a Jenkins server.
Run
/bot [-h|--help]to print this help message.See details below for each supported subcommand.
Details
run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]Launch build/test pipelines. All previously running jobs will be killed.
--reuse-test (optional)pipeline-id(OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.--disable-reuse-test(OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.--disable-fail-fast(OPTIONAL) : Disable fail fast on build/tests/infra failures.--skip-test(OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.--stage-list "A10-PyTorch-1, xxx"(OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.--gpu-type "A30, H100_PCIe"(OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.--test-backend "pytorch, cpp"(OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.--only-multi-gpu-test(OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.--disable-multi-gpu-test(OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.--add-multi-gpu-test(OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.--post-merge(OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx"(OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".--detailed-log(OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.--debug(OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in thestage-listparameter to access the appropriate container environment. Note: Does NOT update GitHub check status.For guidance on mapping tests to stage names, see
docs/source/reference/ci-overview.mdand the
scripts/test_to_stage_mapping.pyhelper.kill
killKill all running builds associated with pull request.
skip
skip --comment COMMENTSkip testing for latest commit on pull request.
--comment "Reason for skipping build/test"is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.reuse-pipeline
reuse-pipelineReuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.