Remove items not relevant to documentation in PR template #234
Conversation
|
@lucyleeow actually, would you mind updating this to match the changes in napari/napari#6092? |
|
Which reminds me: @Czaki, should we update the label checking CI here as well? |
| @@ -1,8 +1,6 @@ | |||
| # Description | |||
| <!-- What does this pull request (PR) do? Why is it necessary? --> | |||
| <!-- Tell us about your new content, improvement, or fix! --> | |||
| <!-- If you can, please add an image, or an animation "An image is worth a thousand words!" --> | |||
| <!-- You can use https://www.cockos.com/licecap/ or similar to create animations --> | |||
There was a problem hiding this comment.
I'm actually not sure about this. In documentation it is even more important to add images, so why not leave this in?
There was a problem hiding this comment.
Ah I read this as add images to the github PR thread - e.g., over in napari, people may put a screenshot/video capture in a comment to demonstrate what their code does. I thought here, if an image is added we could just look at it in the file tab. Though now that I think about it maybe with lfs we won't be able to see the image? (I am really not familiar with lfs).
Maybe it is worth a slight re-wording to clarify what we mean (esp as I think we mean something different when we say this over in napari?)
There was a problem hiding this comment.
Related, do we have documentation on how to deal with lfs when contributing images in docs?
There was a problem hiding this comment.
We are not using lfs anymore, so this shouldn't be a problem. I read this comment as "Please add images to the documentation page you are writing if you can", but maybe I am missing the perspective of the user here - they may understand like you did. So not sure!
There was a problem hiding this comment.
I've updated it slightly and kept this, thanks for your review!
| - [ ] My PR is the minimum possible work for the desired functionality | ||
| - [ ] I have commented my code, particularly in hard-to-understand areas | ||
| - [ ] I have added [alt text](https://webaim.org/techniques/alttext/) to new images included in this PR |
There was a problem hiding this comment.
No strong feelings here, but we did include this thinking of tooling/infrastructure PRs (dealing with sphinx, sphinx-gallery, CI etc).
There was a problem hiding this comment.
Don't feel strongly here either but would like to remove the checklist as I think doc only PRs would be most common and I never know what I am mean to do (with the irrelevant checklist items) in that case.
Maybe a minor re-word also here to make these more relevant E.g., comment code so others know what it achieves
Could we also add a 'if the PR relates to tooling/infrastructure changes:' before the items - because when I make a doc PR the first 2 items seem a bit awkward/confusing to me?
I suggest yes. |
@brisvag done! |
Description
Remove items not relevant to documentation in PR template
Also makes check list just a list, like napari/napari#6092 did
Type of change
References
Final checklist: