Suggestions to hopefully improve PR checklist #2
Conversation
|
@eiriksm |
|
Yeah its not super easy. Maybe we should do something like "Check this item if this is not applicable"? |
| - [ ] 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) |
There was a problem hiding this comment.
I especially like this change. Nice 👌
| - [ ] 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 --> |
There was a problem hiding this comment.
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?
| - [ ] 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 --> | ||
| - [ ] My code follows the style guidelines of this project. |
There was a problem hiding this comment.
I don't think that should be part of the checklist. That should be completely automatic and part of CI
| - [ ] Can it be deployed automatically? <!-- If manual actions are required, fill in the deploy steps below --> | ||
| - [ ] My code follows the style guidelines of this project. | ||
| - [ ] I have performed a self-review of my code. | ||
| - [ ] This change requires a documentation update and I have updated the documentation. |
There was a problem hiding this comment.
Maybe, if we want to keep it as a checklist, we can do something like
This change requires a documentation update and I have updated the documentation. If not relevant, check this box anyway
| - [ ] My code follows the style guidelines of this project. | ||
| - [ ] I have performed a self-review of my code. | ||
| - [ ] This change requires a documentation update and I have updated the documentation. | ||
| - [ ] This change requires a translation update and I have updated the translations. |
There was a problem hiding this comment.
Maybe same here?
...if not relevant, check this box anyway
Who needs a PR checklist? 😜
Here are some suggestions to kickstart the improvement of the checklist. I think that we should also gather some feedback from the people on why they don't fill the current checklist appropriately, like if they find some difficulties or if somehting is not clear enough, etc, that will allow us to improve it imo.
I think we should aim to something simple and clear as possible.