Make PR template form clearer and more concise - #1703
Conversation
dylannelson
left a comment
There was a problem hiding this comment.
Overall more concise, less bullets, and more explanation in the form of comments instead. Good changes for the user in mind.
maybe this could be removed, as it's basically just saying "please listen and read" lol:
please follow instructions below while filling out this form, and read the etiquette reminders at the bottom.
Not a serious difference either way. Thanks for the change!
Thank you for the review! Re: the note at the top, I agree and I did feel a bit silly while writing it, but I think it might be okay (I certainly don't mind if people just skim past it). I mainly included it because I wanted to move the "Thank you for opening a PR" message to the top, to show some appreciation to contributors before asking to complete lots of steps/instructions. But, just putting "Thank you for opening a PR" felt too weird / too blunt to me…. Secondary benefit is to highlight that the etiquette reminders exist; before they were extremely easy to ignore / not notice at all. (I will leave as-is for now, but I'm definitely open to more feedback about it!) |
rajeeja
left a comment
There was a problem hiding this comment.
Thanks for doing this — we're at 17 checkboxes today and nobody in our ecosystem is close (xarray 7, matplotlib 5, dask 3). This gets us to 10, which is the right direction, but even after this PR we'd be at 81 lines. Let's cut more:
- Expected Usage code block → comment
- Etiquette reminders → CONTRIBUTING.md
- Docs folder paths → CONTRIBUTING.md
- Merge Documentation and Examples boxes
- Target ~20 lines, xarray-sized
While I agree with some of the above, I think it's a good idea to keep some form of code visualized & required, not as a comment. When we receive bug reports, the code itself can make a massive difference in debugging and time to solve, so making it just a delete example and paste code here makes it far easier for a user and much less likely for a user to skim over and ignore. I say we keep the code block at least in some form I also feel like 20 lines would not be possible without removing multiple entire sections, or making it an absolute wall of dense, hard to read text |
Thank you for taking a closer look! To me it feels a bit too aggressive to make that many cuts all at once. What would your thoughts be about merging the changes here for now as "an improvement but not necessarily the final stage", with the intent to revisit in a few days/weeks/months? Right now I think this PR is scoped as "rephrase things on the PR form to be clearer and more concise, mostly by removing redundancies, not by making major content cuts." I think implementing your suggestions would re-scope it to "make major cuts to the PR form", which feels to me like it would fit better as a new issue/PR. In particular I would definitely want to hear @erogluorhan's thoughts before making major cuts like that. Sidenote, I might be concerned that replacing small helpful reminders with instructions like "go read the contributor's guide" would actually raise the barrier to entry, instead of lowering it, because it requires following a link to a separate location, and the Contributor’s Guide is quite long…. Also, what do you mean by "Merge Documentation and Examples boxes"? I also agree with @dylannelson's point about the Expected Usage code block; it has been extremely useful for me in my PRs which have needed it, and it hasn't really been much of an imposition on my time or efforts to just delete that section completely when it isn't needed (honestly it feels kind of refreshing to delete an entire section, not bad at all). |
|
Fair on the code block — keep it. On merging now and revisiting later: in my experience that revisit never happens, template cleanup is exactly the kind of low-priority thing that sits forever, so whatever we can agree to cut now is worth more than a follow-up issue. By "merge Documentation and Examples" I meant collapsing those two sections into one "docs and examples updated" box — that plus the docs folder paths moving to CONTRIBUTING would be a small, uncontroversial win to land here. |
and remove those details from pr template form. pr template form now links to the relevant part of that docs page instead of providing all details inline. Also, merges Documentation with Examples section in pr checklist, and edits examples language slightly.
Fair about follow-up, I'm not sure if it would actually end up happening or not. I agree that merging those sections and moving the docs folder paths details into the Contributor's guide are both likely uncontroversial (now that I understand what you meant by "merge Documentation and Examples"). I have implemented those changes in my latest commit! |
way to go, approving :) |
Closes #1696
Overview
Makes the PR template form more concise.
Minor expansions of this PR's scope:
[N/A]…? This also helps reduce mental load; instead of "consider if the whole section applies, then consider if each point applies" it becomes simply "consider if each point applies".I hope these changes would all improve contributor experience. But, they are all based on my subjective opinions and intuitions. Please let me know if you have any questions or concerns, or would prefer different phrasing!
PR Checklist
General
AI Disclosure
AI Usage: GitHub Copilot suggestions show up inline in my editor and maybe influenced my thought processes here, but I don't remember if I actually accepted any of them directly.