Added a Pull Request Template #17856
Conversation
No, it's not 🤗 I'd be forced to do the same every time. We could put this in a comment ( |
.github/pull_request_template.md
Outdated
| Link to the accepted Trac ticket for this PR. All PRs except docs or tests changes need an accepted ticket. New features need community consensus. | ||
|
|
||
| # Tests | ||
| Briefly describe any tests you have added or updated to test the changes introduced by the PR |
There was a problem hiding this comment.
Is that generally expected? I feel like having the tests in the checklist is good enough – asking people to describe the tests they've written feels like a sure-fire way to get the paragraph deleted entirely.
There was a problem hiding this comment.
I agree 👍
Additionally when we're also draining people's cognitive resources by adding too much stuff to read so repetitive things should be removed.
Less is more :)
.github/pull_request_template.md
Outdated
| Briefly describe any tests you have added or updated to test the changes introduced by the PR | ||
|
|
||
| # Checklist | ||
| - [ ] I have updated the Trac ticket’s flags |
There was a problem hiding this comment.
As Trac isn't super intuitive, I think it would be helpful to tell users what to do exactly (ie set "has patch" on the ticket).
There was a problem hiding this comment.
| - [ ] I have updated the Trac ticket’s flags | |
| - [ ] I have updated the "has patch" Trac ticket |
.github/pull_request_template.md
Outdated
| - [ ] I have added or updated tests to test the changes I made | ||
| - [ ] I have added or updated relevant documentation (if applicable) | ||
|
|
||
| # More details |
There was a problem hiding this comment.
I'm not sure this section is a good idea, tbh, for multiple reasons:
- When you open a PR, GitHub shows you a sidebar linking to the contributing.rst, which in turn links to the contribution docs.
- The "Working with git" documentation is probably not the most relevant once people are already about to open a PR
- The PR template is opened in an editable text box, so using Markdown format is cumbersome, as you either have to manually select the link, or switch to the preview panel to click it
- IME, the template should be as short as possible (so people don't just delete it entirely), and only contain things that should end up in the PR text – not least of all because you can search PRs, and leaving this section in would potentially clutter up the search.
There was a problem hiding this comment.
I'm not sure this section is a good idea, tbh, for multiple reasons:
* When you open a PR, GitHub shows you a sidebar linking to the contributing.rst, which in turn links [to the contribution docs](https://docs.djangoproject.com/en/dev/internals/contributing/).
While this link is helpful, I think it too broad and unspecific about the stage the author is at this point.
* The "Working with git" documentation is probably not the most relevant once people are already about to open a PR
Good point about the git docs, though there are a few sections at the bottom of that page that are very relevant for when proposing a PR and then following up during reviews. It describes rebasing and other relevant processes. I'm proposing that we tailor the link to these sections instead.
* The PR template is opened in an editable text box, so using Markdown format is cumbersome, as you either have to manually select the link, or switch to the preview panel to click it * IME, the template should be as short as possible (so people don't just delete it entirely), and only contain things that should end up in the PR text – not least of all because you can search PRs, and leaving this section in would potentially clutter up the search.
We have many contributors not following the process of having a ticket, adding tests, adding docs, setting the trac flags for a PR. I believe that they are not following the expected procedure because they don't know about it (not because they ignore it). And I think that they don't know about it because they got lost in the contributing docs (I've been there!). More so, I think that while is likely that (new?) contributors are familiar with GH, they are not with other systems or may be discouraged by the need to jump between the docs site, the trac site, and this site.
With this information, do you think there is something we could do to improve the contributor experience and, consequently, help reviewers receive more complete PRs?
There was a problem hiding this comment.
The PR template is opened in an editable text box, so using Markdown format is cumbersome, as you either have to manually select the link, or switch to the preview panel to click it
I think I agree with @rixx in that this information might be better placed elsewhere, since having doc links in a PR template can make the template overcrowded with info, and the fact that even opening the links from the template is a bit cumbersome is a good point.
I agree that it's easy to get lost in the contributing docs, but maybe that's something we need to fix separately?
OK great. So it sounds like the target audience of this is not regular contributors. @erosselli could you help explain who's experience (or maybe persona of user) you are aiming to improve with this change? I think with that in mind it will aid review of the content. Thanks for your efforts here! |
|
I think the checklist is a good thing to have and would also be useful for experienced contributors as a reminder. |
There was a problem hiding this comment.
Thank you @erosselli! I think this is a solid step forward improving the contributor experience and to help reviewers receive more complete PRs.
Please do not forget to wrap lines at 79cols!
.github/pull_request_template.md
Outdated
| Briefly describe any tests you have added or updated to test the changes introduced by the PR | ||
|
|
||
| # Checklist | ||
| - [ ] I have updated the Trac ticket’s flags |
There was a problem hiding this comment.
| - [ ] I have updated the Trac ticket’s flags | |
| - [ ] I have updated the "has patch" Trac ticket |
.github/pull_request_template.md
Outdated
| - [ ] I have added or updated tests to test the changes I made | ||
| - [ ] I have added or updated relevant documentation (if applicable) | ||
|
|
||
| # More details |
There was a problem hiding this comment.
I'm not sure this section is a good idea, tbh, for multiple reasons:
* When you open a PR, GitHub shows you a sidebar linking to the contributing.rst, which in turn links [to the contribution docs](https://docs.djangoproject.com/en/dev/internals/contributing/).
While this link is helpful, I think it too broad and unspecific about the stage the author is at this point.
* The "Working with git" documentation is probably not the most relevant once people are already about to open a PR
Good point about the git docs, though there are a few sections at the bottom of that page that are very relevant for when proposing a PR and then following up during reviews. It describes rebasing and other relevant processes. I'm proposing that we tailor the link to these sections instead.
* The PR template is opened in an editable text box, so using Markdown format is cumbersome, as you either have to manually select the link, or switch to the preview panel to click it * IME, the template should be as short as possible (so people don't just delete it entirely), and only contain things that should end up in the PR text – not least of all because you can search PRs, and leaving this section in would potentially clutter up the search.
We have many contributors not following the process of having a ticket, adding tests, adding docs, setting the trac flags for a PR. I believe that they are not following the expected procedure because they don't know about it (not because they ignore it). And I think that they don't know about it because they got lost in the contributing docs (I've been there!). More so, I think that while is likely that (new?) contributors are familiar with GH, they are not with other systems or may be discouraged by the need to jump between the docs site, the trac site, and this site.
With this information, do you think there is something we could do to improve the contributor experience and, consequently, help reviewers receive more complete PRs?
Hi @smithdc1 , I think that having a PR template can be helpful in different cases. The first case I thought of is to help new contributors, so they can know how to structure a PR & what kind of information to include in the description. The checklist can help make sure they have done everything needed (it's easy to forget things :) ). I'm a rather new contributor myself and when I opened my first PR I was surprised to see there was no template; it felt a bit daunting to write the whole thing from scratch. The checklist also servers as a reminder for all contributors and I think could help reduce the number of contributors not following "due process" (marking flags in the ticket, adding tests or docs when needed, etc). |
|
Hi @erosselli, thank you for taking the time to write such a considered response! I think the new contributor experience is super important. On the topic of new contributors, they currently receive this message [1] when opening a new PR. What's your thoughts on how these two interact? Just as a small example, I wonder about the duplication. I'd fill in the checklist in the template and then have a message to refer me to the checklist (which I've already filled in)? [1] https://github.com/django/django/blob/main/.github%2Fworkflows%2Fnew_contributor_pr.yml |
From my experience, the "first time contributor" message, while valuable, has a few drawbacks:
Do you mean in the "new contributor message"? If yes, this makes sense, do not duplicate the "branch review checklist".
|
Maybe the message can be updated to refer to the checklist in the PR, as a reminder, or simply remove that part of the message altogether? |
|
So should we remove the new contributor message if this is merged? 🤔 |
I think we can leave it, maybe rephrasing or removing some of the more repetitive parts (mainly the instructions to set the flag on the ticket & the reminder to link the ticket in the description, since the template already covers that?). There's parts of it that I think are still relevant, for example the part directing the person to ask any questions they may have on the forum :) I also think the "Welcome aboard ⛵ " message is nice :) |
nessita
force-pushed
the
pull_request_template_proposal
branch
from
9f81f9f
to
5493cba
Compare
There was a problem hiding this comment.
Thank you @erosselli, this looks good! I pushed some edits, will land soon.
Co-authored-by: Natalia <124304+nessita@users.noreply.github.com> Co-authored-by: Paolo Melchiorre <paolo@melchiorre.org> Co-authored-by: Adam Johnson <me@adamj.eu>
nessita
force-pushed
the
pull_request_template_proposal
branch
from
5493cba
to
8febdde
Compare
|
I think I just became the first to fill in the template in #17936 🥳 |
Amazing! Any initial feedback about the template? How did it "feel"? |
|
Overall good, it feels like the right length. Two things I noticed when using:
|
@adamchainz I finally got to this item in my ToDo! Here the PR: #18098 |
Description
As discussed in this Django Forum thread, this is a proposal for a Pull Request Template for Django. Any feedback or comments are welcome!