followup: rather than
`What is the best way to verify this PR? `
what about
`What effect should I expect to see from this change?`
On Wed, Feb 28, 2018 at 10:25 AM, Dan Kirkwood <[email protected]> wrote:
> +1 on keeping it short and to the point...
>
> 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
>> > >>> >
>> > >>>
>> > >>
>> > >>
>> >
>>
>
>
