MNT: Simplify PR template#32100
Conversation
Closes matplotlib#32093. This is radically reduced to the essence. We many discuss the single quality check box. But I felt ticking one box may still be a reasonable checkpoint to rethink the PR. If quality check was only informative text, it would likely be ignored much more. My fear is rather that people do not understand that they should tick the box. Note: this is a template I myself would be willing to use :)
In last week's meeting we discussed maybe using one of the AI review bots and @melissawm is doing a survey. The reason I think we can get rid of the checkboxes all together is b/c I'm guessing the coding guidelines are more or less a skills document. |
| Thank you so much for your PR! To help us review your contribution, please check | ||
| out the development guide https://matplotlib.org/devdocs/devel/index.html | ||
| --> | ||
| <!-- Thank you for your PR! Please check our development guide https://matplotlib.org/devdocs/devel/index.html --> |
There was a problem hiding this comment.
| <!-- Thank you for your PR! Please check our development guide https://matplotlib.org/devdocs/devel/index.html --> | |
| <!-- Thank you for your PR! Please check that your PR complies with our development guide https://matplotlib.org/devdocs/devel/index.html --> |
|
|
||
| ## PR summary | ||
| <!-- Please describe the pull request, using the questions below as guidance, and link to any relevant issues and PRs: | ||
| <!-- Please describe what problem is solved and why you chose this solution. --> |
There was a problem hiding this comment.
| <!-- Please describe what problem is solved and why you chose this solution. --> | |
| <!-- Please describe what issue is resolved and why you chose this solution. --> |
If they are solving an issue then hopefully it's a prompt to link that issue, and means the same thing even if there's no underlying issue.
| - Why is this change necessary? | ||
| - What problem does it solve? | ||
| - What is the reasoning for this implementation? |
There was a problem hiding this comment.
can this language maybe get moved to https://matplotlib.org/devdocs/devel/contribute.html#first-contributions b/c that's what it was aimed for
There was a problem hiding this comment.
Uh, oh that section has a verbatim copy of the PR template - will also need and upate, but not today anymore.
There was a problem hiding this comment.
Uh, oh that section has a verbatim copy of the PR template - will also need and upate, but not today anymore.
That dropdown is an include to this file specifically so that they're always in sync:
matplotlib/doc/devel/contribute.rst
Lines 328 to 332 in 102a1d2
| - [ ] *New Features* and *API Changes* are noted with a [directive and release note](https://matplotlib.org/devdocs/devel/api_changes.html#announce-changes-deprecations-and-new-features) | ||
| - [ ] Documentation complies with [general](https://matplotlib.org/devdocs/devel/document.html#write-rest-pages) and [docstring](https://matplotlib.org/devdocs/devel/document.html#write-docstrings) guidelines | ||
| ## PR quality check | ||
| <!-- Make sure to conform with the project standards. Please check the following box! --> |
There was a problem hiding this comment.
I don' t think this needs the free text explanation of the checkbox
| <!-- Make sure to conform with the project standards. Please check the following box! --> |
There was a problem hiding this comment.
Without explanation and with the shorted checkbox text, will users still understand that they should tick the box?
There was a problem hiding this comment.
I think the folks who aren't checking just aren't checking, but you can also make it sound like an attestation?
- [ ] I affirm that this PR complies with [matplotlib pull request guidelines](https://matplotlib.org/devdocs/devel/pr_guide.html#pull-request-guidelines)| the recommended next step seems overly demanding, if you would like help in | ||
| addressing a reviewer's comments, or if you have been waiting too long to hear | ||
| back on your PR.--> | ||
| - [ ] I am aware of and have followed the [matplotlib pull request guidelines](https://matplotlib.org/devdocs/devel/pr_guide.html#pull-request-guidelines) |
There was a problem hiding this comment.
| - [ ] I am aware of and have followed the [matplotlib pull request guidelines](https://matplotlib.org/devdocs/devel/pr_guide.html#pull-request-guidelines) | |
| - [ ] this PR complies with [matplotlib pull request guidelines](https://matplotlib.org/devdocs/devel/pr_guide.html#pull-request-guidelines) |
How does this relate to the PR template? Do you mean using it to evaluate the PR description? |
That it'd be doing the "this PR complies w/ project standards" check just like the CI already handles most of the other checks. I don't know who the checkbox is for that isn't handled w/ the nudge at the top. |
Closes #32093.
This is radically reduced to the essence.
We many discuss the single quality check box. But I felt ticking one box may still be a reasonable checkpoint to rethink the PR. If quality check was only informative text, it would likely be ignored much more. My fear is rather that people do not understand that they should tick the box.
Note: this is a template I myself would be willing to use :)