-
Notifications
You must be signed in to change notification settings - Fork 0
Suggestions to hopefully improve PR checklist #2
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,23 @@ | ||
| **References** <!-- Link to the Jira task --> | ||
| PROJ-XXX | ||
|
|
||
| ### Changed <!-- What changed with this PR --> | ||
| ### Type of change | ||
| - [ ] 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 --> | ||
|
|
||
| ### Visuals <!-- Add here some screenshots --> | ||
| ### Description of the change with screenshots (if possible) | ||
| <!-- Describe here what changed with this PR --> | ||
|
|
||
| ### How can this be validated? <!-- Instructions for QA --> | ||
| ### Validation steps | ||
| <!-- Instructions for QA --> | ||
|
|
||
| ### Pull request checklist | ||
| - [ ] Can it be deployed automatically? _If manual actions are required:_ did you describe the deploy steps? | ||
| - [ ] Is the documentation updated, if it makes sense? <!-- Check this also if no updating is needed --> | ||
| - [ ] Are necessary translations added/updated? <!-- Check this even if no translations are being changed --> | ||
| - [ ] Did you update sanitization routines, where personal information is handled? | ||
| - [ ] Can it be deployed automatically? <!-- If manual actions are required, fill in the deploy steps below --> | ||
|
Contributor
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'm not sure I agree with moving it to a comment like this. I don't know how other people use this, but personally I use the checklist to click the different checkboxes when it's in rendered mode. In this mode, the comments does not appear, and then I would not be reminded about filling it out. What do you think? |
||
| - [ ] My code follows the style guidelines of this project. | ||
|
Contributor
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 don't think that should be part of the checklist. That should be completely automatic and part of CI |
||
| - [ ] I have performed a self-review of my code. | ||
| - [ ] This change requires a documentation update and I have updated the documentation. | ||
|
Contributor
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. Maybe, if we want to keep it as a checklist, we can do something like |
||
| - [ ] This change requires a translation update and I have updated the translations. | ||
|
Contributor
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. Maybe same here? |
||
| - [ ] Did you update sanitization routines, where personal information is handled? <!-- Delete if not applicable --> | ||
|
|
||
| ### Deploy steps <!-- Delete this if the deploy is automatic --> | ||
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.
I especially like this change. Nice 👌