Create pull_request_template.md #773
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
That's a nice initiative, thanks! |
| Please delete options that are not relevant. | ||
|
|
||
| - [ ] Bug fix (non-breaking change which fixes an issue) | ||
| - [ ] New feature (non-breaking change which adds functionality) | ||
| - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) | ||
| - [ ] This change requires a documentation update | ||
| - [ ] This change requires a translation update |
There was a problem hiding this comment.
I'm not convinced a list like this is very practical — I would drop the "Type of change" section altogether unless we actually have a different process for different types of change. For example, "This change requires a documentation update" is useful in theory but unless someone painstakingly goes through PRs looking for those that need a doc update, the checkbox ends up being little more than an administrative annoyance. Just my opinion, of course.
| - [ ] This change requires a documentation update | ||
| - [ ] This change requires a translation update | ||
|
|
||
| # How Has This Been Tested? |
There was a problem hiding this comment.
I would reword this into "How can this be tested?" or just "How to test" — this would put the focus on more testing being done by reviewers/testers rather than relying too much on the author having covered all bases.
In my opinion it's fine to ask for help if you can't thoroughly test a change you're proposing.
| # Checklist: | ||
|
|
||
| - [ ] My code follows the style guidelines of this project | ||
| - [ ] I have performed a self-review of my code | ||
| - [ ] I have commented my code, particularly in hard-to-understand areas | ||
| - [ ] I have made corresponding changes to the documentation | ||
| - [ ] My changes generate no new warnings | ||
| - [ ] I have added tests that prove my fix is effective or that my feature works | ||
| - [ ] New and existing unit tests pass locally with my changes | ||
| - [ ] Any dependent changes have been merged and published in downstream modules |
There was a problem hiding this comment.
I feel similarly about this checklist as with the "Type of change" list — it feels a bit infantilizing to have to check every single box to assert you've been a good contributor.
I would suggest a link to the contributing guide (which may need to be beefed up) here instead.
Motivated by a recent PR which was merged without thorough testing on different operating systems, I propose to provide a Pull Request template (similar to the issue templates we already have).
It shall remind the creator and the reviewer of usual things to keep track of.
Plus, it provides checklists to document e.g. which operating systems has already been tested with, or if the change causes additional changes in documentation (manual) or translations.
It's basically based on a template I found on the internet - contributions are highly welcome. So it's a draft PR for now.