PR Titles

[Jacob Barrett <jb...@pivotal.io>]

Please use meaningful PR titles. When browsing the PRs or reading the notifications, one shouldn’t have to decode your cryptic title. Consider the title as the first line of you commit message.

-Jake

Re: PR Titles

[Mark Hanson <mh...@pivotal.io>]

The reason I prefer the rules not apply in draft is quite simple, people sometimes use the PR pipeline to test stuff.  Its easy enough to just put in GEODE-<Blah> but sometimes it just “Test” because there is no GEODE associated.  Either way I don’t care enough to debate. I will abide by what the devs agree to. 

I disagree with statement about do not review. I think it is particularly useful. I have had people review PRs that were labelled Do not review. So grayed out icon is not sufficient. People wouldn’t label that way if it did not serve a purpose.

I would like to say that people need to be more flexible and supportive of opinions that differ from our own opinions because the alternative is an uninviting environment. That said, I am not that attached to any of my opinions above. They are merely suggestions taken or not.

Thanks,
Mark

> On Mar 2, 2020, at 10:47 AM, Ernest Burghardt <eb...@pivotal.io> wrote:
> 
> I agree with Jake and do appreciate everyone using the "DRAFT" feature, one
> downfall of that is this is not visible unless on Github; i.e. if you see
> an email for a PR you won't know its in "DRAFT" mode until you go look at
> it...
> 
> a related part of this, to me, seems to be that the PR checklist is a bit
> out of date... much of it is now automatically enforced (awesome), but
> arguably the most important part of the checklist "[*] Have you written or
> updated unit tests to verify your changes?" is buried at the bottom and is
> an important signal being lost in the noise...
> 
> On Mon, Mar 2, 2020 at 10:12 AM Jacob Barrett <jb...@pivotal.io> wrote:
> 
>> 
>> 
>>> On Mar 2, 2020, at 10:10 AM, Mark Hanson <mh...@pivotal.io> wrote:
>>> 
>>> If I may add one caveat. If a pull request is in a ready to review state
>> … <Rules apply>. If it is prefaced with Do Not Review: then <Rules do not
>> apply>.
>> -1
>> Use the “Draft” state of the PR and use good titles from the beginning.
>> Using “DO NO REVIEW” in the title is super annoying now that GitHub has a
>> feature for draft PRs.
>> 
>> 

Re: PR Titles

[Ernest Burghardt <eb...@pivotal.io>]

I agree with Jake and do appreciate everyone using the "DRAFT" feature, one
downfall of that is this is not visible unless on Github; i.e. if you see
an email for a PR you won't know its in "DRAFT" mode until you go look at
it...

a related part of this, to me, seems to be that the PR checklist is a bit
out of date... much of it is now automatically enforced (awesome), but
arguably the most important part of the checklist "[*] Have you written or
updated unit tests to verify your changes?" is buried at the bottom and is
an important signal being lost in the noise...

On Mon, Mar 2, 2020 at 10:12 AM Jacob Barrett <jb...@pivotal.io> wrote:

>
>
> > On Mar 2, 2020, at 10:10 AM, Mark Hanson <mh...@pivotal.io> wrote:
> >
> > If I may add one caveat. If a pull request is in a ready to review state
> … <Rules apply>. If it is prefaced with Do Not Review: then <Rules do not
> apply>.
>  -1
> Use the “Draft” state of the PR and use good titles from the beginning.
> Using “DO NO REVIEW” in the title is super annoying now that GitHub has a
> feature for draft PRs.
>
>

Re: PR Titles

[Jacob Barrett <jb...@pivotal.io>]

> On Mar 2, 2020, at 10:10 AM, Mark Hanson <mh...@pivotal.io> wrote:
> 
> If I may add one caveat. If a pull request is in a ready to review state … <Rules apply>. If it is prefaced with Do Not Review: then <Rules do not apply>.
 -1
Use the “Draft” state of the PR and use good titles from the beginning. Using “DO NO REVIEW” in the title is super annoying now that GitHub has a feature for draft PRs.

Re: PR Titles

[Udo Kohlmeyer <ud...@apache.com>]

I thought PR titles are generated from the git comment.

So, I'm expecting it to have the following format: "GEODE-XXX: ........"

Glancing at the current PR list, I do see that there is at least 1 PR 
that does not follow that guideline.

We also don't want to be overly prescriptive, so maybe if we find a PR 
that does not meet that condition, we comment on that PR, asking for a 
change or title and description to make it easier to understand what 
that PR is for.

As for another check on the checklist... Well, I don't always see that 
the current set are correctly filled in.. So, it just becomes another 
check that some contributors will not check.

@Mark, I struggle to understand why one would submit a PR if it is not 
to be reviewed or included in the project. Sounds like unnecessary noise 
to me and should be discouraged

--Udo

On 3/2/20 10:10 AM, Mark Hanson wrote:
> If I may add one caveat. If a pull request is in a ready to review state … <Rules apply>. If it is prefaced with Do Not Review: then <Rules do not apply>.
>
> Thanks,
> Mark
>
>> On Mar 2, 2020, at 9:30 AM, Alexander Murmann <am...@apache.org> wrote:
>>
>>> Don't we have a published checklist or guideline or something for this
>>> already?  Including stuff like 'always prefix the PR title with a JIRA
>>> ticket #,' etc?
>>
>> Yes, we do. However, it only request the addition of the JIRA ticket #. It
>> does not call out having a descriptive title.
>>
>> I like the idea of adding a point to the checklist to call this out.
>>
>>
>> On Mon, Mar 2, 2020 at 9:02 AM Blake Bender <bb...@pivotal.io> wrote:
>>
>>> Don't we have a published checklist or guideline or something for this
>>> already?  Including stuff like 'always prefix the PR title with a JIRA
>>> ticket #,' etc?
>>>
>>> On Mon, Mar 2, 2020 at 8:47 AM Owen Nichols <on...@pivotal.io> wrote:
>>>
>>>> +1
>>>>
>>>> It’s easy to fix too!
>>>>
>>>> On Mon, Mar 2, 2020 at 8:34 AM Jacob Barrett <jb...@pivotal.io>
>>> wrote:
>>>>> Please use meaningful PR titles. When browsing the PRs or reading the
>>>>> notifications, one shouldn’t have to decode your cryptic title.
>>> Consider
>>>>> the title as the first line of you commit message.
>>>>>
>>>>> -Jake
>>>>>
>>>>>

Re: PR Titles

[Mark Hanson <mh...@pivotal.io>]

If I may add one caveat. If a pull request is in a ready to review state … <Rules apply>. If it is prefaced with Do Not Review: then <Rules do not apply>.

Thanks,
Mark

> On Mar 2, 2020, at 9:30 AM, Alexander Murmann <am...@apache.org> wrote:
> 
>> 
>> Don't we have a published checklist or guideline or something for this
>> already?  Including stuff like 'always prefix the PR title with a JIRA
>> ticket #,' etc?
> 
> 
> Yes, we do. However, it only request the addition of the JIRA ticket #. It
> does not call out having a descriptive title.
> 
> I like the idea of adding a point to the checklist to call this out.
> 
> 
> On Mon, Mar 2, 2020 at 9:02 AM Blake Bender <bb...@pivotal.io> wrote:
> 
>> Don't we have a published checklist or guideline or something for this
>> already?  Including stuff like 'always prefix the PR title with a JIRA
>> ticket #,' etc?
>> 
>> On Mon, Mar 2, 2020 at 8:47 AM Owen Nichols <on...@pivotal.io> wrote:
>> 
>>> +1
>>> 
>>> It’s easy to fix too!
>>> 
>>> On Mon, Mar 2, 2020 at 8:34 AM Jacob Barrett <jb...@pivotal.io>
>> wrote:
>>> 
>>>> Please use meaningful PR titles. When browsing the PRs or reading the
>>>> notifications, one shouldn’t have to decode your cryptic title.
>> Consider
>>>> the title as the first line of you commit message.
>>>> 
>>>> -Jake
>>>> 
>>>> 
>>> 
>> 

Re: PR Titles

[Alexander Murmann <am...@apache.org>]

>
> Don't we have a published checklist or guideline or something for this
> already?  Including stuff like 'always prefix the PR title with a JIRA
> ticket #,' etc?


Yes, we do. However, it only request the addition of the JIRA ticket #. It
does not call out having a descriptive title.

I like the idea of adding a point to the checklist to call this out.


On Mon, Mar 2, 2020 at 9:02 AM Blake Bender <bb...@pivotal.io> wrote:

> Don't we have a published checklist or guideline or something for this
> already?  Including stuff like 'always prefix the PR title with a JIRA
> ticket #,' etc?
>
> On Mon, Mar 2, 2020 at 8:47 AM Owen Nichols <on...@pivotal.io> wrote:
>
> > +1
> >
> > It’s easy to fix too!
> >
> > On Mon, Mar 2, 2020 at 8:34 AM Jacob Barrett <jb...@pivotal.io>
> wrote:
> >
> > > Please use meaningful PR titles. When browsing the PRs or reading the
> > > notifications, one shouldn’t have to decode your cryptic title.
> Consider
> > > the title as the first line of you commit message.
> > >
> > > -Jake
> > >
> > >
> >
>

Re: PR Titles

[Blake Bender <bb...@pivotal.io>]

Don't we have a published checklist or guideline or something for this
already?  Including stuff like 'always prefix the PR title with a JIRA
ticket #,' etc?

On Mon, Mar 2, 2020 at 8:47 AM Owen Nichols <on...@pivotal.io> wrote:

> +1
>
> It’s easy to fix too!
>
> On Mon, Mar 2, 2020 at 8:34 AM Jacob Barrett <jb...@pivotal.io> wrote:
>
> > Please use meaningful PR titles. When browsing the PRs or reading the
> > notifications, one shouldn’t have to decode your cryptic title. Consider
> > the title as the first line of you commit message.
> >
> > -Jake
> >
> >
>

Re: PR Titles

[Owen Nichols <on...@pivotal.io>]

+1

It’s easy to fix too!

On Mon, Mar 2, 2020 at 8:34 AM Jacob Barrett <jb...@pivotal.io> wrote:

> Please use meaningful PR titles. When browsing the PRs or reading the
> notifications, one shouldn’t have to decode your cryptic title. Consider
> the title as the first line of you commit message.
>
> -Jake
>
>