[MNT] Enhance PR template with detailed checklist items#270
Conversation
Updated the pull request template to clarify self-review and testing requirements.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis change modifies the Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The checklist items for tests are a bit awkward as independent checkboxes; consider rewording them into a single, mutually exclusive bullet (e.g., “Tests covering these changes: [ ] added new tests / [ ] already covered by existing tests”) to make the intent clearer.
- There is a small wording issue in the new checklist item “All changes already under covered under existing tests”; you probably want to drop the extra “under” (e.g., “already covered by existing tests”).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The checklist items for tests are a bit awkward as independent checkboxes; consider rewording them into a single, mutually exclusive bullet (e.g., “Tests covering these changes: [ ] added new tests / [ ] already covered by existing tests”) to make the intent clearer.
- There is a small wording issue in the new checklist item “All changes already under covered under existing tests”; you probably want to drop the extra “under” (e.g., “already covered by existing tests”).
## Individual Comments
### Comment 1
<location path=".github/PULL_REQUEST_TEMPLATE.md" line_range="16" />
<code_context>
+- [ ] Tests pass locally
- [ ] I have commented my code in hard-to-understand areas, and provided or updated docstrings as needed
+- [ ] I have added tests that cover the changes, or
+- [ ] All changes already under covered under existing tests
+
+If applicable:
</code_context>
<issue_to_address>
**issue (typo):** Fix the duplicated "under" and missing verb in this checkbox line.
This line has a typo: "All changes already under covered under existing tests." It should read something like "All changes are already covered under existing tests" or "All changes are already covered by existing tests."
```suggestion
- [ ] All changes are already covered under existing tests
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/PULL_REQUEST_TEMPLATE.md (2)
21-22: Broaden the bot disclosure wording.
created autonomouslyis pretty narrow, so PRs largely authored by a bot but lightly edited by a human will likely go undisclosed. If the goal is prioritization and expectation-setting, the checkbox should cover material bot/agent authorship, not just fully autonomous submissions.Proposed wording
-Extra context: -- [ ] This PR and the commits have been created autonomously by a bot/agent. +Extra context: +- [ ] This PR or its commits were created wholly or substantially by a bot/agent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/PULL_REQUEST_TEMPLATE.md around lines 21 - 22, Replace the narrow checkbox text "This PR and the commits have been created autonomously by a bot/agent." with broader wording that captures material bot involvement (e.g., "This PR and/or a significant portion of its commits were authored by a bot/agent" or "This PR contains substantial content authored by a bot/agent") so lightly-edited bot-generated PRs are disclosed; update the checkbox line in .github/PULL_REQUEST_TEMPLATE.md (match the existing checkbox syntax) and adjust any adjacent copy if necessary to preserve clarity and checklist formatting.
15-16: Make the test coverage statement unambiguous.Using two independent checkboxes for an
orcondition lets authors check both or neither. Consider collapsing this into one item that requires the author to state which case applies.Proposed wording
-- [ ] I have added tests that cover the changes, or -- [ ] All changes are already covered under existing tests +- [ ] Test coverage is sufficient for this change (new tests added, or existing tests already cover it)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/PULL_REQUEST_TEMPLATE.md around lines 15 - 16, Replace the two ambiguous checklist lines "- [ ] I have added tests that cover the changes, or" and "- [ ] All changes are already covered under existing tests" with a single unambiguous item that requires authors to explicitly state which case applies (for example: "- [ ] I confirm either I have added tests that cover the changes OR all changes are already covered by existing tests — please indicate which: (added | existing)"), so authors cannot check both or neither.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/PULL_REQUEST_TEMPLATE.md:
- Around line 21-22: Replace the narrow checkbox text "This PR and the commits
have been created autonomously by a bot/agent." with broader wording that
captures material bot involvement (e.g., "This PR and/or a significant portion
of its commits were authored by a bot/agent" or "This PR contains substantial
content authored by a bot/agent") so lightly-edited bot-generated PRs are
disclosed; update the checkbox line in .github/PULL_REQUEST_TEMPLATE.md (match
the existing checkbox syntax) and adjust any adjacent copy if necessary to
preserve clarity and checklist formatting.
- Around line 15-16: Replace the two ambiguous checklist lines "- [ ] I have
added tests that cover the changes, or" and "- [ ] All changes are already
covered under existing tests" with a single unambiguous item that requires
authors to explicitly state which case applies (for example: "- [ ] I confirm
either I have added tests that cover the changes OR all changes are already
covered by existing tests — please indicate which: (added | existing)"), so
authors cannot check both or neither.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b16de9b9-2e0d-495c-8da9-228c80ec945b
📒 Files selected for processing (1)
.github/PULL_REQUEST_TEMPLATE.md
1 participant
Updated the pull request template to clarify self-review and testing requirements.
The main motivation was that I prefer bots to self-identify. It's not that hard to spot typically, and correct changes might normally still be accepted, but it can help with prioritization or managing expectation on response times.