-
-
Notifications
You must be signed in to change notification settings - Fork 101
Update contribution guidelines in pull request template #834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
julianz-
wants to merge
1
commit into
cherrypy:main
Choose a base branch
from
julianz-:update-pr-template
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+49
β19
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,14 @@ | ||
| β **What kind of change does this PR introduce?** | ||
| <!-- | ||
| Thank you for your contribution! | ||
| Please use our PR template β PRs that strip it will not be reviewed. | ||
|
|
||
| Before submitting, please ensure you have: | ||
| - Added a change log fragment in docs/changelog-fragments.d/ (see the link in the checklist below) | ||
| - Written commits with a clear title and a body explaining the why | ||
| - Avoided letting an AI agent strip or overwrite the fields below | ||
| --> | ||
|
|
||
| ## What kind of change does this PR introduce? | ||
|
|
||
| * [ ] π bug fix | ||
| * [ ] π£ feature | ||
|
|
@@ -7,41 +17,57 @@ | |
| * [ ] π refactoring | ||
| * [ ] π₯ other | ||
|
|
||
| π **What is the related issue number (starting with `#`)** | ||
|
|
||
| <!-- Are there any issues opened that will be resolved by merging this change? --> | ||
| Resolves #<!-- issue number here --> | ||
| ## What do these changes do? | ||
|
|
||
| β **What is the current behavior?** (You can also link to an open issue here) | ||
| <!-- Please give a short brief about these changes. --> | ||
|
|
||
| ## Are there changes in behavior for the user? | ||
|
|
||
| <!-- Outline any notable behavior for the end users. --> | ||
|
|
||
| β **What is the new behavior (if this is a feature change)?** | ||
| ## Is it a substantial burden for the maintainers to support this? | ||
|
|
||
| <!-- | ||
| Stop right there! Pause. Just for a minute... Can you think of anything | ||
| obvious that would complicate the ongoing development of this project? | ||
|
|
||
| Try to consider if you'd be able to maintain it throughout the next | ||
| 5 years. Does it seem viable? Tell us your thoughts! We'd very much | ||
| love to hear what the consequences of merging this patch might be... | ||
|
|
||
| π **Other information**: | ||
| This will help us assess if your change is something we'd want to | ||
| entertain early in the review process. Thank you in advance! | ||
| --> | ||
|
|
||
| ## Related issue number | ||
|
|
||
| <!-- Will this resolve any open issues? --> | ||
| <!-- Remember to prefix with 'Fixes' if it closes an issue (e.g. 'Fixes #123'). --> | ||
|
|
||
| π **Contribution checklist:** | ||
|
|
||
| (If you're a first-timer, check out | ||
| [this guide on making great pull requests][making a lovely PR]) | ||
| ## Checklist | ||
|
|
||
| * [ ] I wrote descriptive pull request text above | ||
| * [ ] The PR relates to *only* one subject with a clear title | ||
| and description in grammatically correct, complete sentences | ||
| * [ ] I think the code is well written | ||
| * [ ] I wrote [good commit messages] | ||
| * [ ] I have [squashed related commits together][related squash] after | ||
| the changes have been approved | ||
| * [ ] Unit tests for the changes exist | ||
| * [ ] Integration tests for the changes exist (if applicable) | ||
| * [ ] I used the same coding conventions as the rest of the project | ||
| * [ ] The new code doesn't generate linter offenses | ||
| * [ ] Documentation reflects the changes | ||
| * [ ] The PR relates to *only* one subject with a clear title | ||
| and description in grammatically correct, complete sentences | ||
|
|
||
| * [ ] Project documentation (in `docs/`) and inline docstrings reflect the changes | ||
| * [ ] My commits each have a descriptive title and a body explaining the why | ||
| (see [guide][good commit messages]) | ||
| * [ ] I have added a [changelog][changelog-fragments] entry | ||
| (can be done once the PR number is known) | ||
| * [ ] I'm planning to [squash related commits together][related squash] | ||
| before final merge | ||
| * [ ] I have read the [contribution guide][CONTRIBUTING] and the | ||
| [code of conduct][CoC] | ||
|
|
||
| [CONTRIBUTING]: https://cheroot.cherrypy.dev/en/latest/contributing/guidelines/ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the future, we should render the CoC in the docs too.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added in #835 |
||
| [CoC]: https://github.com/cherrypy/cheroot?tab=coc-ov-file#readme | ||
| [changelog-fragments]: | ||
| https://github.com/cherrypy/cheroot/tree/main/docs/changelog-fragments.d#readme | ||
| [good commit messages]: http://chris.beams.io/posts/git-commit/ | ||
| [making a lovely PR]: https://mtlynch.io/code-review-love/ | ||
| [related squash]: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Updated the pull request template with consistent heading structure, | ||
| a change log entry checkbox linking to the change log guidelines, | ||
| and improved commit message guidance | ||
| -- by :user:`julianz-`. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pre-existing template used emoji icons for βtitlesβ but didn't use the Markdown heading syntax. And your new changes have it vice versa. This is inconsistent and we should pick one.
@julianz- I recommend that you copy this file into an experimental empty repository and see how it works by simulating the process you expect within that playground to catch such inconsistencies and other UX/DX quirks that may be difficult to spot in a PR review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see https://github.com/julianz-/cheroot-pr-template-test/pull/1