What about: *Describe your PR:* _(copy/paste from changeset comments is encouraged!)_
*If there's no update to CHANGELOG.md, why not?* *Does this break backward compatibility?* *Quick Checklist:* - [ ] Each commit message tells you everything you need to know about the change, including an issue number if appropriate. (Squashing can help with this.) - [ ] This PR does *not* fix a serious security flaw. (Read more: www.apache.org/security ) - [ ] Tests are complete. - [ ] Docs are updated. ... The idea is that the form only asks for information that the reviewer can't trivially get elsewhere, or is of particular importance. I think you're right, though: short is more important than detailed. Is there anything else that could be taken out? I do think that "How do I verify this?" isn't a great question for the PR template. If the answer isn't immediately obvious, then the commit messages haven't described the feature in enough detail. The PR will disappear, but the commit messages stick around. A discussion of what the feature does in the comments of a PR won't be useful to someone trying to determine what the commit was trying to accomplish a year down the line when it's unclear whether the behaviour was intended or accidental. On Wed, Feb 28, 2018 at 10:14 AM, Jeremy Mitchell <[email protected]> wrote: > Chris, I really wanted the PR template to be less daunting and super short > and to the point. It's intention is to give a super quick summary of what's > included in this PR to help the merger... > > Example: > > #### What does this PR do? Is there a related Github issue? > > "See Issue #1245" or "This PR cascade deletes all delivery service regexes > when a delivery service is deleted" > > #### What is the best way to verify this PR? <-- IMO this is really > important for the merger so I know how to "test" or "verify" the > functionality. > > Hit the DELETE /api/1.3/deliveryservices/:id endpoint and ensure all > entries in the deliveryservice_regex table are deleted for that delivery > service. > > #### Does your PR include any of the following? > > - [ ] Tests > - [ ] Documentation > - [X] CHANGELOG.md entry > > ^^ I wasn't trying to imply that those last things were required. I just > wanted to provide a checklist that might be helpful for the contributer and > the merger. For example, I always for get to look for a CHANGELOG.md > entry... > > Jeremy > > > On Tue, Feb 27, 2018 at 9:47 PM, Chris Lemmons <[email protected]> wrote: > >> If there's a relevant GitHub issue, that should be noted in the >> check-in comment, I think. Same for "what does it do?" I don't usually >> want to spell out steps for someone to verify my stuff because those >> are the steps that I took to verify it. The PR is so you can see the >> things I didn't see. And the commit list will make the presence of >> tests, documentation, and a changelog entry really obvious. >> >> Taking yours and reformatting a bit, what if we did something like this? >> >> ... >> >> *Describe your PR:* _(copy/paste from changeset comments is encouraged!)_ >> >> >> >> *Quick Checklist:* >> >> - [ ] Each commit message tells you everything you need to know about >> the change. (Squashing can help with this.) >> - [ ] If applicable, the commit message mentions the appropriate issue >> number. >> - [ ] This PR does *not* fix a serious security flaw. (Read more: >> www.apache.org/security ) >> - [ ] Automatic code formatters (like gofmt) have been run. >> >> *Tests:* >> >> - [ ] Are not necessary. >> - [ ] Would be helpful, but aren't in this PR. >> - [ ] Are present, but incomplete. >> - [ ] Are included. >> >> *Doc updates:* >> >> - [ ] Are not necessary. >> - [ ] Would be helpful, but aren't in this PR. >> - [ ] Are present, but incomplete. >> - [ ] Are included. >> >> *If there's no update to CHANGELOG.md, why not?* >> >> *Does this break backward compatibility?* >> >> *Is there anyone specific that ought to take a look at this?* >> >> ... >> >> We want to be friendly to committers, while still getting good >> information for checking PRs. I could be easily convinced that the >> "Tests" or "Doc updates" sections in there are too long, but I think >> it should be clear that a potential committer can offer up some code >> without hitting 100% on tests, docs, and such. >> >> On Tue, Feb 27, 2018 at 1:24 PM, Jeremy Mitchell <[email protected]> >> wrote: >> > How about something like this for a PR template? >> > >> > #### What does this PR do? Is there a relevant Github issue? >> > >> > >> > #### What is the best way to verify this PR? >> > >> > >> > #### Does your PR include any of the following? >> > >> > - [ ] Tests >> > - [ ] Documentation >> > - [ ] CHANGELOG.md entry >> > >> > On Tue, Feb 27, 2018 at 10:46 AM, Jeremy Mitchell <[email protected] >> > >> > wrote: >> > >> >> With an issue and/or pr template, we will have 6/6 items checked: >> >> >> >> https://github.com/apache/incubator-trafficcontrol/community >> >> >> >> I actually think PR templates would be quite helpful. As a >> >> committer/merger, it would be nice to know what problem the PR is >> solving >> >> and how to verify the functionality. A PR template could also help >> >> contributors ensure that their PRs are complete. I.e. does this PR >> includes >> >> tests, documentation, etc. >> >> >> >> I'll take a stab at a couple of templates and run them by the group. >> >> >> >> Jeremy >> >> >> >> On Wed, Jan 31, 2018 at 1:10 PM, Chris Lemmons <[email protected]> >> wrote: >> >> >> >>> I'm +1 on Issue Templates, for sure. I don't know that PR templates >> >>> are quite as critical, but it might be nice to have a reminder in >> >>> there about verifying that the changelog is updated if necessary and >> >>> documentation for new features is present. If the PR Template >> >>> overwrites the default comment that you get from the commit body, it >> >>> might be more annoying than valuable, though. >> >>> >> >>> I'm also +1 on hiding these particular files in a .github directory. >> >>> Unlike CONTRIBUTING and README, they don't provide all that much >> >>> benefit for a new person looking for stuff to read. >> >>> >> >>> On Wed, Jan 31, 2018 at 12:17 PM, Durfey, Ryan < >> [email protected]> >> >>> wrote: >> >>> > Always +1 on standardization and consistency >> >>> > >> >>> > I still want to circle back and setup project/kanbans for organizing >> >>> tickets in Github. >> >>> > >> >>> > Ryan Durfey M | 303-524-5099 >> >>> > CDN Support (24x7): 866-405-2993 or [email protected]<mailto: >> >>> [email protected]> >> >>> > >> >>> > From: Dewayne Richardson <[email protected]> >> >>> > Reply-To: "[email protected]" < >> >>> [email protected]> >> >>> > Date: Wednesday, January 31, 2018 at 11:15 AM >> >>> > To: "[email protected]" < >> >>> [email protected]> >> >>> > Subject: Github PR/Issues Format Templates >> >>> > >> >>> > I was working through the go-swagger repo and found a bug. I >> submitted >> >>> a >> >>> > new issue and found this interesting approach I think the TC github >> >>> should >> >>> > adopt, "Issue and PR Templates". I think the main value here is >> >>> > consistency in our PRs/Issues and user friendly prompts to say "these >> >>> are >> >>> > the data points we need to help you solve your issue". >> >>> > >> >>> > Working example: >> >>> > https://github.com/go-swagger/go-swagger/issues/new >> >>> > >> >>> > Github Doc on how to implement templates: >> >>> > https://github.com/blog/2111-issue-and-pull-request-templates >> >>> > >> >>> > If we think it's a good idea, then I'll respond with some examples >> for >> >>> > Issues and PR's that we can discuss. >> >>> > >> >>> > -Dew >> >>> > >> >>> >> >> >> >> >>
