You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@reef.apache.org by Markus Weimer <ma...@weimo.de> on 2015/01/23 00:52:32 UTC
Pull request hygiene
Hi,
I have been merging several pull requests recently that did not adhere
to the guidelines we gave ourselves. Most notably, the following things
didn't happen:
1. The PR was not squashed to a single commit when it was opened.
2. The one commit didn't have a message in accordance with our
contributor's guidelines.
3. The PR wasn't announced on JIRA.
Given that we don't follow our own rules, I'd like to know why and what
we want to do about it?
For instance We could drop (1) and (2) in the above list, which would
just move work from the contributor to the committer. This is exactly
what happened for the aforementioned PRs.
Number 3 above seems crucial to me. If we have PRs that completely
sideline JIRA, we risk loosing community consensus along the way.
Thoughts? Comments?
Markus
Re: Pull request hygiene
Posted by Byung-Gon Chun <bg...@gmail.com>.
Markus,
I think we should keep the guidelines.
#3 is very important, I agree.
The procedure for contributors takes effort a bit but they get the credit
for their contribution. We should not put much work to git committers. Just
a thought.
-Gon
On Fri, Jan 23, 2015 at 8:52 AM, Markus Weimer <ma...@weimo.de> wrote:
> Hi,
>
> I have been merging several pull requests recently that did not adhere
> to the guidelines we gave ourselves. Most notably, the following things
> didn't happen:
>
> 1. The PR was not squashed to a single commit when it was opened.
> 2. The one commit didn't have a message in accordance with our
> contributor's guidelines.
> 3. The PR wasn't announced on JIRA.
>
> Given that we don't follow our own rules, I'd like to know why and what
> we want to do about it?
>
> For instance We could drop (1) and (2) in the above list, which would
> just move work from the contributor to the committer. This is exactly
> what happened for the aforementioned PRs.
>
> Number 3 above seems crucial to me. If we have PRs that completely
> sideline JIRA, we risk loosing community consensus along the way.
>
> Thoughts? Comments?
>
> Markus
>
--
Byung-Gon Chun
RE: Pull request hygiene
Posted by "Julia Wang (QIUHE)" <Qi...@microsoft.com>.
No matter who does final squash, we don't need to create a new PR, right?
-----Original Message-----
From: John Yang [mailto:johnyangk@gmail.com]
Sent: Monday, January 26, 2015 10:59 PM
To: dev@reef.incubator.apache.org
Subject: Re: Pull request hygiene
If a new PR needs to be created when a contributor does the final squash, then I am also in favor of the committer doing it.
Also, I think it would be good to explicitly state this rule in the related articles of our wiki:
https://cwiki.apache.org/confluence/display/REEF/Contributing
https://cwiki.apache.org/confluence/display/REEF/Committer+Guide
In the contributor guide, we can explicitly state that squashing the commits into a single commit before filing a PR is mandatory and the final squash will be carried out by the committer(or the contributor, if we decide otherwise).
In the committer guide, we can make a note to ourselves to reject PRs that do not meet the guidelines.
John
On Tue, Jan 27, 2015 at 4:09 AM, Markus Weimer <ma...@weimo.de> wrote:
> I'd be in favor of the committer doing the final squash.
>
> Markus
>
>
>
> On 2015-01-26 10:04, Julia Wang (QIUHE) wrote:
> > Just to clarify:
> >
> > A PR starts with only one commit by contributor.
> > After final CR, the reviewer informs contributor to do final squash?
> >
> > Thanks,
> > Julia
> >
> > -----Original Message-----
> > From: Byung-Gon Chun [mailto:bgchun@gmail.com]
> > Sent: Sunday, January 25, 2015 4:31 PM
> > To: dev@reef.incubator.apache.org
> > Subject: Re: Pull request hygiene
> >
> > +1
> > Let's reject PRs that do not meet the formal guidelines.
> >
> >
> > On Mon, Jan 26, 2015 at 4:32 AM, Markus Weimer <ma...@weimo.de> wrote:
> >
> >> Hi,
> >>
> >> I don't think we should have the contributor do the final squash.
> >> It would mean that they have to create another PR, I believe. This
> >> makes bookkeeping much harder. If the committer does it, the code
> >> goes straight to Apache's git repository and can close the PR.
> >>
> >> In this thread, we seem to agree that the current split of work
> >> between contributor on the wiki is what we /want/ to do, yet we
> >> somehow
> don't.
> >> Shall we reject PRs on formal grounds then?
> >>
> >> Markus
> >>
> >>
> >> On 2015-01-24 00:29, John Yang wrote:
> >>> Great, that sounds good to me.
> >>>
> >>> John
> >>> On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)"
> >>> <Qi...@microsoft.com>
> >>> wrote:
> >>>
> >>>> I agree - having the PR start with only one commit - this is
> >>>> should be easy to achieve for contributor.
> >>>>
> >>>>
> >>>>
> >>>> After complete code review cycle and ready to merge, if
> >>>> necessary, committer can request contributor to do one more squash.
> >>>>
> >>>>
> >>>>
> >>>> -----Original Message-----
> >>>> From: Markus Weimer [mailto:markus@weimo.de]
> >>>> Sent: Friday, January 23, 2015 8:33 AM
> >>>> To: dev@reef.incubator.apache.org
> >>>> Subject: Re: Pull request hygiene
> >>>>
> >>>>
> >>>>
> >>>> Hi,
> >>>>
> >>>>
> >>>>
> >>>> it goes without saying that assigning blame was far from my
> intentions.
> >>>>
> >>>> There is just a certain amount of work that can be done either on
> >>>> the contributor or the committer side of the process. I
> >>>> understand that we
> >> all
> >>>> think that making the comitters' life easier is important. From
> >>>> that
> >> angle,
> >>>> having the contributor do the *final* squash isn't all that
> >>>> important,
> >> as
> >>>> the work there is limited.
> >>>>
> >>>>
> >>>>
> >>>> However, I find having the PR start with only one commit very helpful.
> >>>>
> >>>> It allows us to capture a good commit message right then, which
> >>>> GitHub will use as the PR description. It always picks the first
> >>>> commit for
> >> that,
> >>>> hence the need to squash. And it also makes the editing of the
> >>>> final
> >> squash
> >>>> much, much easier.
> >>>>
> >>>>
> >>>>
> >>>> Regarding item 3 on my list: We all are in agreement on this one.
> >>>> Hence, we can be obnoxious then and reject PRs that have no JIRA
> >>>> reference :-)
> >>>>
> >>>>
> >>>>
> >>>> Markus
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On 2015-01-22 17:57, John Yang wrote:
> >>>>
> >>>>> Hi,
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>> I didn’t adhere to 1 and 2. Sorry about that.
> >>>>
> >>>>>
> >>>>
> >>>>> I also think it is a good idea to notify the contributor once
> >>>>> the PR is
> >>>> good to merge, so that he/she does not forget.
> >>>>
> >>>>>
> >>>>
> >>>>> 3 should definitely be kept.
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>> Thanks,
> >>>>
> >>>>> John
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com
> <mailto:
> >>>> bgchun@gmail.com>> wrote:
> >>>>
> >>>>>>
> >>>>
> >>>>>> The option sounds good to me.
> >>>>
> >>>>>>
> >>>>
> >>>>>> ---
> >>>>
> >>>>>> Byung-Gon Chun
> >>>>
> >>>>>>
> >>>>
> >>>>>> Sent from my phone
> >>>>
> >>>>>>
> >>>>
> >>>>>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE)
> >>>>>> <Qiuhe.Wang@microsoft.com
> >>>> <ma...@microsoft.com>> 작성:
> >>>>
> >>>>>>
> >>>>
> >>>>>>> We should definitely keep 3 as it gives us a document to trace
> >>>>>>> why we
> >>>> made the change.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> 1 is little bit hard. Even if we squash before making a new
> >>>>>>> pull
> >>>> request, after receiving comments and we make some changes, would
> >>>> you
> >> like
> >>>> to keep the delta or you would like to squash them into one commit?
> >>>> And
> >> as
> >>>> we could back force a few times during the code review, or
> >>>> sometimes may just make very minor format change, squash each
> >>>> time seems quite troublesome.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> One option is when the code is ready to merge, the committer
> >>>>>>> and
> >>>> inform the contributor to do onetime squash. The contributor can
> >>>> use
> >> this
> >>>> opportunity to put proper comments as it can be changed during
> >>>> the code review as well.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Thanks,
> >>>>
> >>>>>>> Julia
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> -----Original Message-----
> >>>>
> >>>>>>> From: Markus Weimer [mailto:markus@weimo.de]
> >>>>
> >>>>>>> Sent: Thursday, January 22, 2015 3:53 PM
> >>>>
> >>>>>>> To: REEF Developers Mailinglist
> >>>>
> >>>>>>> Subject: Pull request hygiene
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Hi,
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> I have been merging several pull requests recently that did
> >>>>>>> not
> >> adhere
> >>>> to the guidelines we gave ourselves. Most notably, the following
> >>>> things didn't happen:
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> 1. The PR was not squashed to a single commit when it was opened.
> >>>>
> >>>>>>> 2. The one commit didn't have a message in accordance with our
> >>>>
> >>>>>>> contributor's guidelines.
> >>>>
> >>>>>>> 3. The PR wasn't announced on JIRA.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Given that we don't follow our own rules, I'd like to know why
> >>>>>>> and
> >>>> what we want to do about it?
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> For instance We could drop (1) and (2) in the above list,
> >>>>>>> which would
> >>>> just move work from the contributor to the committer. This is
> >>>> exactly
> >> what
> >>>> happened for the aforementioned PRs.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Number 3 above seems crucial to me. If we have PRs that
> >>>>>>> completely
> >>>> sideline JIRA, we risk loosing community consensus along the way.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Thoughts? Comments?
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Markus
> >>>>
> >>>>>
> >>>>
> >>>
> >>
> >
> >
> >
> > --
> > Byung-Gon Chun
> >
>
Re: Pull request hygiene
Posted by Markus Weimer <ma...@weimo.de>.
Yes, but I am pretty sure we'd loose much of the usable history on
GitHub's PR page if we do that.
Markus
On 2015-01-26 23:56, Julia Wang (QIUHE) wrote:
> I mean we only need to squash to merge all the commits into one instead of creating a new PR.
>
> -----Original Message-----
> From: Julia Wang (QIUHE)
> Sent: Monday, January 26, 2015 11:56 PM
> To: dev@reef.incubator.apache.org
> Subject: RE: Pull request hygiene
>
> No matter who does final squash, we don't need to create a new PR, right?
>
> -----Original Message-----
> From: John Yang [mailto:johnyangk@gmail.com]
> Sent: Monday, January 26, 2015 10:59 PM
> To: dev@reef.incubator.apache.org
> Subject: Re: Pull request hygiene
>
> If a new PR needs to be created when a contributor does the final squash, then I am also in favor of the committer doing it.
>
> Also, I think it would be good to explicitly state this rule in the related articles of our wiki:
> https://cwiki.apache.org/confluence/display/REEF/Contributing
> https://cwiki.apache.org/confluence/display/REEF/Committer+Guide
>
> In the contributor guide, we can explicitly state that squashing the commits into a single commit before filing a PR is mandatory and the final squash will be carried out by the committer(or the contributor, if we decide otherwise).
>
> In the committer guide, we can make a note to ourselves to reject PRs that do not meet the guidelines.
>
>
> John
>
> On Tue, Jan 27, 2015 at 4:09 AM, Markus Weimer <ma...@weimo.de> wrote:
>
>> I'd be in favor of the committer doing the final squash.
>>
>> Markus
>>
>>
>>
>> On 2015-01-26 10:04, Julia Wang (QIUHE) wrote:
>>> Just to clarify:
>>>
>>> A PR starts with only one commit by contributor.
>>> After final CR, the reviewer informs contributor to do final squash?
>>>
>>> Thanks,
>>> Julia
>>>
>>> -----Original Message-----
>>> From: Byung-Gon Chun [mailto:bgchun@gmail.com]
>>> Sent: Sunday, January 25, 2015 4:31 PM
>>> To: dev@reef.incubator.apache.org
>>> Subject: Re: Pull request hygiene
>>>
>>> +1
>>> Let's reject PRs that do not meet the formal guidelines.
>>>
>>>
>>> On Mon, Jan 26, 2015 at 4:32 AM, Markus Weimer <ma...@weimo.de> wrote:
>>>
>>>> Hi,
>>>>
>>>> I don't think we should have the contributor do the final squash.
>>>> It would mean that they have to create another PR, I believe. This
>>>> makes bookkeeping much harder. If the committer does it, the code
>>>> goes straight to Apache's git repository and can close the PR.
>>>>
>>>> In this thread, we seem to agree that the current split of work
>>>> between contributor on the wiki is what we /want/ to do, yet we
>>>> somehow
>> don't.
>>>> Shall we reject PRs on formal grounds then?
>>>>
>>>> Markus
>>>>
>>>>
>>>> On 2015-01-24 00:29, John Yang wrote:
>>>>> Great, that sounds good to me.
>>>>>
>>>>> John
>>>>> On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)"
>>>>> <Qi...@microsoft.com>
>>>>> wrote:
>>>>>
>>>>>> I agree - having the PR start with only one commit - this is
>>>>>> should be easy to achieve for contributor.
>>>>>>
>>>>>>
>>>>>>
>>>>>> After complete code review cycle and ready to merge, if
>>>>>> necessary, committer can request contributor to do one more squash.
>>>>>>
>>>>>>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Markus Weimer [mailto:markus@weimo.de]
>>>>>> Sent: Friday, January 23, 2015 8:33 AM
>>>>>> To: dev@reef.incubator.apache.org
>>>>>> Subject: Re: Pull request hygiene
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>>
>>>>>> it goes without saying that assigning blame was far from my
>> intentions.
>>>>>>
>>>>>> There is just a certain amount of work that can be done either on
>>>>>> the contributor or the committer side of the process. I
>>>>>> understand that we
>>>> all
>>>>>> think that making the comitters' life easier is important. From
>>>>>> that
>>>> angle,
>>>>>> having the contributor do the *final* squash isn't all that
>>>>>> important,
>>>> as
>>>>>> the work there is limited.
>>>>>>
>>>>>>
>>>>>>
>>>>>> However, I find having the PR start with only one commit very helpful.
>>>>>>
>>>>>> It allows us to capture a good commit message right then, which
>>>>>> GitHub will use as the PR description. It always picks the first
>>>>>> commit for
>>>> that,
>>>>>> hence the need to squash. And it also makes the editing of the
>>>>>> final
>>>> squash
>>>>>> much, much easier.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Regarding item 3 on my list: We all are in agreement on this one.
>>>>>> Hence, we can be obnoxious then and reject PRs that have no JIRA
>>>>>> reference :-)
>>>>>>
>>>>>>
>>>>>>
>>>>>> Markus
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2015-01-22 17:57, John Yang wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>
>>>>>>>
>>>>>>
>>>>>>>
>>>>>>
>>>>>>> I didn’t adhere to 1 and 2. Sorry about that.
>>>>>>
>>>>>>>
>>>>>>
>>>>>>> I also think it is a good idea to notify the contributor once
>>>>>>> the PR is
>>>>>> good to merge, so that he/she does not forget.
>>>>>>
>>>>>>>
>>>>>>
>>>>>>> 3 should definitely be kept.
>>>>>>
>>>>>>>
>>>>>>
>>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>
>>>>>>> John
>>>>>>
>>>>>>>
>>>>>>
>>>>>>>
>>>>>>
>>>>>>>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com
>> <mailto:
>>>>>> bgchun@gmail.com>> wrote:
>>>>>>
>>>>>>>>
>>>>>>
>>>>>>>> The option sounds good to me.
>>>>>>
>>>>>>>>
>>>>>>
>>>>>>>> ---
>>>>>>
>>>>>>>> Byung-Gon Chun
>>>>>>
>>>>>>>>
>>>>>>
>>>>>>>> Sent from my phone
>>>>>>
>>>>>>>>
>>>>>>
>>>>>>>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE)
>>>>>>>> <Qiuhe.Wang@microsoft.com
>>>>>> <ma...@microsoft.com>> 작성:
>>>>>>
>>>>>>>>
>>>>>>
>>>>>>>>> We should definitely keep 3 as it gives us a document to trace
>>>>>>>>> why we
>>>>>> made the change.
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> 1 is little bit hard. Even if we squash before making a new
>>>>>>>>> pull
>>>>>> request, after receiving comments and we make some changes, would
>>>>>> you
>>>> like
>>>>>> to keep the delta or you would like to squash them into one commit?
>>>>>> And
>>>> as
>>>>>> we could back force a few times during the code review, or
>>>>>> sometimes may just make very minor format change, squash each
>>>>>> time seems quite troublesome.
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> One option is when the code is ready to merge, the committer
>>>>>>>>> and
>>>>>> inform the contributor to do onetime squash. The contributor can
>>>>>> use
>>>> this
>>>>>> opportunity to put proper comments as it can be changed during
>>>>>> the code review as well.
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> Thanks,
>>>>>>
>>>>>>>>> Julia
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>
>>>>>>>>> From: Markus Weimer [mailto:markus@weimo.de]
>>>>>>
>>>>>>>>> Sent: Thursday, January 22, 2015 3:53 PM
>>>>>>
>>>>>>>>> To: REEF Developers Mailinglist
>>>>>>
>>>>>>>>> Subject: Pull request hygiene
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> Hi,
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> I have been merging several pull requests recently that did
>>>>>>>>> not
>>>> adhere
>>>>>> to the guidelines we gave ourselves. Most notably, the following
>>>>>> things didn't happen:
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> 1. The PR was not squashed to a single commit when it was opened.
>>>>>>
>>>>>>>>> 2. The one commit didn't have a message in accordance with our
>>>>>>
>>>>>>>>> contributor's guidelines.
>>>>>>
>>>>>>>>> 3. The PR wasn't announced on JIRA.
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> Given that we don't follow our own rules, I'd like to know why
>>>>>>>>> and
>>>>>> what we want to do about it?
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> For instance We could drop (1) and (2) in the above list,
>>>>>>>>> which would
>>>>>> just move work from the contributor to the committer. This is
>>>>>> exactly
>>>> what
>>>>>> happened for the aforementioned PRs.
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> Number 3 above seems crucial to me. If we have PRs that
>>>>>>>>> completely
>>>>>> sideline JIRA, we risk loosing community consensus along the way.
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> Thoughts? Comments?
>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>>>> Markus
>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Byung-Gon Chun
>>>
>>
RE: Pull request hygiene
Posted by "Julia Wang (QIUHE)" <Qi...@microsoft.com>.
I mean we only need to squash to merge all the commits into one instead of creating a new PR.
-----Original Message-----
From: Julia Wang (QIUHE)
Sent: Monday, January 26, 2015 11:56 PM
To: dev@reef.incubator.apache.org
Subject: RE: Pull request hygiene
No matter who does final squash, we don't need to create a new PR, right?
-----Original Message-----
From: John Yang [mailto:johnyangk@gmail.com]
Sent: Monday, January 26, 2015 10:59 PM
To: dev@reef.incubator.apache.org
Subject: Re: Pull request hygiene
If a new PR needs to be created when a contributor does the final squash, then I am also in favor of the committer doing it.
Also, I think it would be good to explicitly state this rule in the related articles of our wiki:
https://cwiki.apache.org/confluence/display/REEF/Contributing
https://cwiki.apache.org/confluence/display/REEF/Committer+Guide
In the contributor guide, we can explicitly state that squashing the commits into a single commit before filing a PR is mandatory and the final squash will be carried out by the committer(or the contributor, if we decide otherwise).
In the committer guide, we can make a note to ourselves to reject PRs that do not meet the guidelines.
John
On Tue, Jan 27, 2015 at 4:09 AM, Markus Weimer <ma...@weimo.de> wrote:
> I'd be in favor of the committer doing the final squash.
>
> Markus
>
>
>
> On 2015-01-26 10:04, Julia Wang (QIUHE) wrote:
> > Just to clarify:
> >
> > A PR starts with only one commit by contributor.
> > After final CR, the reviewer informs contributor to do final squash?
> >
> > Thanks,
> > Julia
> >
> > -----Original Message-----
> > From: Byung-Gon Chun [mailto:bgchun@gmail.com]
> > Sent: Sunday, January 25, 2015 4:31 PM
> > To: dev@reef.incubator.apache.org
> > Subject: Re: Pull request hygiene
> >
> > +1
> > Let's reject PRs that do not meet the formal guidelines.
> >
> >
> > On Mon, Jan 26, 2015 at 4:32 AM, Markus Weimer <ma...@weimo.de> wrote:
> >
> >> Hi,
> >>
> >> I don't think we should have the contributor do the final squash.
> >> It would mean that they have to create another PR, I believe. This
> >> makes bookkeeping much harder. If the committer does it, the code
> >> goes straight to Apache's git repository and can close the PR.
> >>
> >> In this thread, we seem to agree that the current split of work
> >> between contributor on the wiki is what we /want/ to do, yet we
> >> somehow
> don't.
> >> Shall we reject PRs on formal grounds then?
> >>
> >> Markus
> >>
> >>
> >> On 2015-01-24 00:29, John Yang wrote:
> >>> Great, that sounds good to me.
> >>>
> >>> John
> >>> On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)"
> >>> <Qi...@microsoft.com>
> >>> wrote:
> >>>
> >>>> I agree - having the PR start with only one commit - this is
> >>>> should be easy to achieve for contributor.
> >>>>
> >>>>
> >>>>
> >>>> After complete code review cycle and ready to merge, if
> >>>> necessary, committer can request contributor to do one more squash.
> >>>>
> >>>>
> >>>>
> >>>> -----Original Message-----
> >>>> From: Markus Weimer [mailto:markus@weimo.de]
> >>>> Sent: Friday, January 23, 2015 8:33 AM
> >>>> To: dev@reef.incubator.apache.org
> >>>> Subject: Re: Pull request hygiene
> >>>>
> >>>>
> >>>>
> >>>> Hi,
> >>>>
> >>>>
> >>>>
> >>>> it goes without saying that assigning blame was far from my
> intentions.
> >>>>
> >>>> There is just a certain amount of work that can be done either on
> >>>> the contributor or the committer side of the process. I
> >>>> understand that we
> >> all
> >>>> think that making the comitters' life easier is important. From
> >>>> that
> >> angle,
> >>>> having the contributor do the *final* squash isn't all that
> >>>> important,
> >> as
> >>>> the work there is limited.
> >>>>
> >>>>
> >>>>
> >>>> However, I find having the PR start with only one commit very helpful.
> >>>>
> >>>> It allows us to capture a good commit message right then, which
> >>>> GitHub will use as the PR description. It always picks the first
> >>>> commit for
> >> that,
> >>>> hence the need to squash. And it also makes the editing of the
> >>>> final
> >> squash
> >>>> much, much easier.
> >>>>
> >>>>
> >>>>
> >>>> Regarding item 3 on my list: We all are in agreement on this one.
> >>>> Hence, we can be obnoxious then and reject PRs that have no JIRA
> >>>> reference :-)
> >>>>
> >>>>
> >>>>
> >>>> Markus
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On 2015-01-22 17:57, John Yang wrote:
> >>>>
> >>>>> Hi,
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>> I didn’t adhere to 1 and 2. Sorry about that.
> >>>>
> >>>>>
> >>>>
> >>>>> I also think it is a good idea to notify the contributor once
> >>>>> the PR is
> >>>> good to merge, so that he/she does not forget.
> >>>>
> >>>>>
> >>>>
> >>>>> 3 should definitely be kept.
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>> Thanks,
> >>>>
> >>>>> John
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com
> <mailto:
> >>>> bgchun@gmail.com>> wrote:
> >>>>
> >>>>>>
> >>>>
> >>>>>> The option sounds good to me.
> >>>>
> >>>>>>
> >>>>
> >>>>>> ---
> >>>>
> >>>>>> Byung-Gon Chun
> >>>>
> >>>>>>
> >>>>
> >>>>>> Sent from my phone
> >>>>
> >>>>>>
> >>>>
> >>>>>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE)
> >>>>>> <Qiuhe.Wang@microsoft.com
> >>>> <ma...@microsoft.com>> 작성:
> >>>>
> >>>>>>
> >>>>
> >>>>>>> We should definitely keep 3 as it gives us a document to trace
> >>>>>>> why we
> >>>> made the change.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> 1 is little bit hard. Even if we squash before making a new
> >>>>>>> pull
> >>>> request, after receiving comments and we make some changes, would
> >>>> you
> >> like
> >>>> to keep the delta or you would like to squash them into one commit?
> >>>> And
> >> as
> >>>> we could back force a few times during the code review, or
> >>>> sometimes may just make very minor format change, squash each
> >>>> time seems quite troublesome.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> One option is when the code is ready to merge, the committer
> >>>>>>> and
> >>>> inform the contributor to do onetime squash. The contributor can
> >>>> use
> >> this
> >>>> opportunity to put proper comments as it can be changed during
> >>>> the code review as well.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Thanks,
> >>>>
> >>>>>>> Julia
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> -----Original Message-----
> >>>>
> >>>>>>> From: Markus Weimer [mailto:markus@weimo.de]
> >>>>
> >>>>>>> Sent: Thursday, January 22, 2015 3:53 PM
> >>>>
> >>>>>>> To: REEF Developers Mailinglist
> >>>>
> >>>>>>> Subject: Pull request hygiene
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Hi,
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> I have been merging several pull requests recently that did
> >>>>>>> not
> >> adhere
> >>>> to the guidelines we gave ourselves. Most notably, the following
> >>>> things didn't happen:
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> 1. The PR was not squashed to a single commit when it was opened.
> >>>>
> >>>>>>> 2. The one commit didn't have a message in accordance with our
> >>>>
> >>>>>>> contributor's guidelines.
> >>>>
> >>>>>>> 3. The PR wasn't announced on JIRA.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Given that we don't follow our own rules, I'd like to know why
> >>>>>>> and
> >>>> what we want to do about it?
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> For instance We could drop (1) and (2) in the above list,
> >>>>>>> which would
> >>>> just move work from the contributor to the committer. This is
> >>>> exactly
> >> what
> >>>> happened for the aforementioned PRs.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Number 3 above seems crucial to me. If we have PRs that
> >>>>>>> completely
> >>>> sideline JIRA, we risk loosing community consensus along the way.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Thoughts? Comments?
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Markus
> >>>>
> >>>>>
> >>>>
> >>>
> >>
> >
> >
> >
> > --
> > Byung-Gon Chun
> >
>
Re: Pull request hygiene
Posted by John Yang <jo...@gmail.com>.
If a new PR needs to be created when a contributor does the final squash,
then I am also in favor of the committer doing it.
Also, I think it would be good to explicitly state this rule in the related
articles of our wiki:
https://cwiki.apache.org/confluence/display/REEF/Contributing
https://cwiki.apache.org/confluence/display/REEF/Committer+Guide
In the contributor guide, we can explicitly state that squashing the
commits into a single commit before filing a PR is mandatory and the final
squash will be carried out by the committer(or the contributor, if we
decide otherwise).
In the committer guide, we can make a note to ourselves to reject PRs that
do not meet the guidelines.
John
On Tue, Jan 27, 2015 at 4:09 AM, Markus Weimer <ma...@weimo.de> wrote:
> I'd be in favor of the committer doing the final squash.
>
> Markus
>
>
>
> On 2015-01-26 10:04, Julia Wang (QIUHE) wrote:
> > Just to clarify:
> >
> > A PR starts with only one commit by contributor.
> > After final CR, the reviewer informs contributor to do final squash?
> >
> > Thanks,
> > Julia
> >
> > -----Original Message-----
> > From: Byung-Gon Chun [mailto:bgchun@gmail.com]
> > Sent: Sunday, January 25, 2015 4:31 PM
> > To: dev@reef.incubator.apache.org
> > Subject: Re: Pull request hygiene
> >
> > +1
> > Let's reject PRs that do not meet the formal guidelines.
> >
> >
> > On Mon, Jan 26, 2015 at 4:32 AM, Markus Weimer <ma...@weimo.de> wrote:
> >
> >> Hi,
> >>
> >> I don't think we should have the contributor do the final squash. It
> >> would mean that they have to create another PR, I believe. This makes
> >> bookkeeping much harder. If the committer does it, the code goes
> >> straight to Apache's git repository and can close the PR.
> >>
> >> In this thread, we seem to agree that the current split of work
> >> between contributor on the wiki is what we /want/ to do, yet we somehow
> don't.
> >> Shall we reject PRs on formal grounds then?
> >>
> >> Markus
> >>
> >>
> >> On 2015-01-24 00:29, John Yang wrote:
> >>> Great, that sounds good to me.
> >>>
> >>> John
> >>> On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)"
> >>> <Qi...@microsoft.com>
> >>> wrote:
> >>>
> >>>> I agree - having the PR start with only one commit - this is should
> >>>> be easy to achieve for contributor.
> >>>>
> >>>>
> >>>>
> >>>> After complete code review cycle and ready to merge, if necessary,
> >>>> committer can request contributor to do one more squash.
> >>>>
> >>>>
> >>>>
> >>>> -----Original Message-----
> >>>> From: Markus Weimer [mailto:markus@weimo.de]
> >>>> Sent: Friday, January 23, 2015 8:33 AM
> >>>> To: dev@reef.incubator.apache.org
> >>>> Subject: Re: Pull request hygiene
> >>>>
> >>>>
> >>>>
> >>>> Hi,
> >>>>
> >>>>
> >>>>
> >>>> it goes without saying that assigning blame was far from my
> intentions.
> >>>>
> >>>> There is just a certain amount of work that can be done either on
> >>>> the contributor or the committer side of the process. I understand
> >>>> that we
> >> all
> >>>> think that making the comitters' life easier is important. From
> >>>> that
> >> angle,
> >>>> having the contributor do the *final* squash isn't all that
> >>>> important,
> >> as
> >>>> the work there is limited.
> >>>>
> >>>>
> >>>>
> >>>> However, I find having the PR start with only one commit very helpful.
> >>>>
> >>>> It allows us to capture a good commit message right then, which
> >>>> GitHub will use as the PR description. It always picks the first
> >>>> commit for
> >> that,
> >>>> hence the need to squash. And it also makes the editing of the
> >>>> final
> >> squash
> >>>> much, much easier.
> >>>>
> >>>>
> >>>>
> >>>> Regarding item 3 on my list: We all are in agreement on this one.
> >>>> Hence, we can be obnoxious then and reject PRs that have no JIRA
> >>>> reference :-)
> >>>>
> >>>>
> >>>>
> >>>> Markus
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On 2015-01-22 17:57, John Yang wrote:
> >>>>
> >>>>> Hi,
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>> I didn’t adhere to 1 and 2. Sorry about that.
> >>>>
> >>>>>
> >>>>
> >>>>> I also think it is a good idea to notify the contributor once the
> >>>>> PR is
> >>>> good to merge, so that he/she does not forget.
> >>>>
> >>>>>
> >>>>
> >>>>> 3 should definitely be kept.
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>> Thanks,
> >>>>
> >>>>> John
> >>>>
> >>>>>
> >>>>
> >>>>>
> >>>>
> >>>>>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com
> <mailto:
> >>>> bgchun@gmail.com>> wrote:
> >>>>
> >>>>>>
> >>>>
> >>>>>> The option sounds good to me.
> >>>>
> >>>>>>
> >>>>
> >>>>>> ---
> >>>>
> >>>>>> Byung-Gon Chun
> >>>>
> >>>>>>
> >>>>
> >>>>>> Sent from my phone
> >>>>
> >>>>>>
> >>>>
> >>>>>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qiuhe.Wang@microsoft.com
> >>>> <ma...@microsoft.com>> 작성:
> >>>>
> >>>>>>
> >>>>
> >>>>>>> We should definitely keep 3 as it gives us a document to trace
> >>>>>>> why we
> >>>> made the change.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> 1 is little bit hard. Even if we squash before making a new pull
> >>>> request, after receiving comments and we make some changes, would
> >>>> you
> >> like
> >>>> to keep the delta or you would like to squash them into one commit?
> >>>> And
> >> as
> >>>> we could back force a few times during the code review, or
> >>>> sometimes may just make very minor format change, squash each time
> >>>> seems quite troublesome.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> One option is when the code is ready to merge, the committer and
> >>>> inform the contributor to do onetime squash. The contributor can
> >>>> use
> >> this
> >>>> opportunity to put proper comments as it can be changed during the
> >>>> code review as well.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Thanks,
> >>>>
> >>>>>>> Julia
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> -----Original Message-----
> >>>>
> >>>>>>> From: Markus Weimer [mailto:markus@weimo.de]
> >>>>
> >>>>>>> Sent: Thursday, January 22, 2015 3:53 PM
> >>>>
> >>>>>>> To: REEF Developers Mailinglist
> >>>>
> >>>>>>> Subject: Pull request hygiene
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Hi,
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> I have been merging several pull requests recently that did not
> >> adhere
> >>>> to the guidelines we gave ourselves. Most notably, the following
> >>>> things didn't happen:
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> 1. The PR was not squashed to a single commit when it was opened.
> >>>>
> >>>>>>> 2. The one commit didn't have a message in accordance with our
> >>>>
> >>>>>>> contributor's guidelines.
> >>>>
> >>>>>>> 3. The PR wasn't announced on JIRA.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Given that we don't follow our own rules, I'd like to know why
> >>>>>>> and
> >>>> what we want to do about it?
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> For instance We could drop (1) and (2) in the above list, which
> >>>>>>> would
> >>>> just move work from the contributor to the committer. This is
> >>>> exactly
> >> what
> >>>> happened for the aforementioned PRs.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Number 3 above seems crucial to me. If we have PRs that
> >>>>>>> completely
> >>>> sideline JIRA, we risk loosing community consensus along the way.
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Thoughts? Comments?
> >>>>
> >>>>>>>
> >>>>
> >>>>>>> Markus
> >>>>
> >>>>>
> >>>>
> >>>
> >>
> >
> >
> >
> > --
> > Byung-Gon Chun
> >
>
Re: Pull request hygiene
Posted by Markus Weimer <ma...@weimo.de>.
I'd be in favor of the committer doing the final squash.
Markus
On 2015-01-26 10:04, Julia Wang (QIUHE) wrote:
> Just to clarify:
>
> A PR starts with only one commit by contributor.
> After final CR, the reviewer informs contributor to do final squash?
>
> Thanks,
> Julia
>
> -----Original Message-----
> From: Byung-Gon Chun [mailto:bgchun@gmail.com]
> Sent: Sunday, January 25, 2015 4:31 PM
> To: dev@reef.incubator.apache.org
> Subject: Re: Pull request hygiene
>
> +1
> Let's reject PRs that do not meet the formal guidelines.
>
>
> On Mon, Jan 26, 2015 at 4:32 AM, Markus Weimer <ma...@weimo.de> wrote:
>
>> Hi,
>>
>> I don't think we should have the contributor do the final squash. It
>> would mean that they have to create another PR, I believe. This makes
>> bookkeeping much harder. If the committer does it, the code goes
>> straight to Apache's git repository and can close the PR.
>>
>> In this thread, we seem to agree that the current split of work
>> between contributor on the wiki is what we /want/ to do, yet we somehow don't.
>> Shall we reject PRs on formal grounds then?
>>
>> Markus
>>
>>
>> On 2015-01-24 00:29, John Yang wrote:
>>> Great, that sounds good to me.
>>>
>>> John
>>> On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)"
>>> <Qi...@microsoft.com>
>>> wrote:
>>>
>>>> I agree - having the PR start with only one commit - this is should
>>>> be easy to achieve for contributor.
>>>>
>>>>
>>>>
>>>> After complete code review cycle and ready to merge, if necessary,
>>>> committer can request contributor to do one more squash.
>>>>
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Markus Weimer [mailto:markus@weimo.de]
>>>> Sent: Friday, January 23, 2015 8:33 AM
>>>> To: dev@reef.incubator.apache.org
>>>> Subject: Re: Pull request hygiene
>>>>
>>>>
>>>>
>>>> Hi,
>>>>
>>>>
>>>>
>>>> it goes without saying that assigning blame was far from my intentions.
>>>>
>>>> There is just a certain amount of work that can be done either on
>>>> the contributor or the committer side of the process. I understand
>>>> that we
>> all
>>>> think that making the comitters' life easier is important. From
>>>> that
>> angle,
>>>> having the contributor do the *final* squash isn't all that
>>>> important,
>> as
>>>> the work there is limited.
>>>>
>>>>
>>>>
>>>> However, I find having the PR start with only one commit very helpful.
>>>>
>>>> It allows us to capture a good commit message right then, which
>>>> GitHub will use as the PR description. It always picks the first
>>>> commit for
>> that,
>>>> hence the need to squash. And it also makes the editing of the
>>>> final
>> squash
>>>> much, much easier.
>>>>
>>>>
>>>>
>>>> Regarding item 3 on my list: We all are in agreement on this one.
>>>> Hence, we can be obnoxious then and reject PRs that have no JIRA
>>>> reference :-)
>>>>
>>>>
>>>>
>>>> Markus
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 2015-01-22 17:57, John Yang wrote:
>>>>
>>>>> Hi,
>>>>
>>>>>
>>>>
>>>>>
>>>>
>>>>> I didn’t adhere to 1 and 2. Sorry about that.
>>>>
>>>>>
>>>>
>>>>> I also think it is a good idea to notify the contributor once the
>>>>> PR is
>>>> good to merge, so that he/she does not forget.
>>>>
>>>>>
>>>>
>>>>> 3 should definitely be kept.
>>>>
>>>>>
>>>>
>>>>>
>>>>
>>>>> Thanks,
>>>>
>>>>> John
>>>>
>>>>>
>>>>
>>>>>
>>>>
>>>>>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com<mailto:
>>>> bgchun@gmail.com>> wrote:
>>>>
>>>>>>
>>>>
>>>>>> The option sounds good to me.
>>>>
>>>>>>
>>>>
>>>>>> ---
>>>>
>>>>>> Byung-Gon Chun
>>>>
>>>>>>
>>>>
>>>>>> Sent from my phone
>>>>
>>>>>>
>>>>
>>>>>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qiuhe.Wang@microsoft.com
>>>> <ma...@microsoft.com>> 작성:
>>>>
>>>>>>
>>>>
>>>>>>> We should definitely keep 3 as it gives us a document to trace
>>>>>>> why we
>>>> made the change.
>>>>
>>>>>>>
>>>>
>>>>>>> 1 is little bit hard. Even if we squash before making a new pull
>>>> request, after receiving comments and we make some changes, would
>>>> you
>> like
>>>> to keep the delta or you would like to squash them into one commit?
>>>> And
>> as
>>>> we could back force a few times during the code review, or
>>>> sometimes may just make very minor format change, squash each time
>>>> seems quite troublesome.
>>>>
>>>>>>>
>>>>
>>>>>>> One option is when the code is ready to merge, the committer and
>>>> inform the contributor to do onetime squash. The contributor can
>>>> use
>> this
>>>> opportunity to put proper comments as it can be changed during the
>>>> code review as well.
>>>>
>>>>>>>
>>>>
>>>>>>> Thanks,
>>>>
>>>>>>> Julia
>>>>
>>>>>>>
>>>>
>>>>>>> -----Original Message-----
>>>>
>>>>>>> From: Markus Weimer [mailto:markus@weimo.de]
>>>>
>>>>>>> Sent: Thursday, January 22, 2015 3:53 PM
>>>>
>>>>>>> To: REEF Developers Mailinglist
>>>>
>>>>>>> Subject: Pull request hygiene
>>>>
>>>>>>>
>>>>
>>>>>>> Hi,
>>>>
>>>>>>>
>>>>
>>>>>>> I have been merging several pull requests recently that did not
>> adhere
>>>> to the guidelines we gave ourselves. Most notably, the following
>>>> things didn't happen:
>>>>
>>>>>>>
>>>>
>>>>>>> 1. The PR was not squashed to a single commit when it was opened.
>>>>
>>>>>>> 2. The one commit didn't have a message in accordance with our
>>>>
>>>>>>> contributor's guidelines.
>>>>
>>>>>>> 3. The PR wasn't announced on JIRA.
>>>>
>>>>>>>
>>>>
>>>>>>> Given that we don't follow our own rules, I'd like to know why
>>>>>>> and
>>>> what we want to do about it?
>>>>
>>>>>>>
>>>>
>>>>>>> For instance We could drop (1) and (2) in the above list, which
>>>>>>> would
>>>> just move work from the contributor to the committer. This is
>>>> exactly
>> what
>>>> happened for the aforementioned PRs.
>>>>
>>>>>>>
>>>>
>>>>>>> Number 3 above seems crucial to me. If we have PRs that
>>>>>>> completely
>>>> sideline JIRA, we risk loosing community consensus along the way.
>>>>
>>>>>>>
>>>>
>>>>>>> Thoughts? Comments?
>>>>
>>>>>>>
>>>>
>>>>>>> Markus
>>>>
>>>>>
>>>>
>>>
>>
>
>
>
> --
> Byung-Gon Chun
>
RE: Pull request hygiene
Posted by "Julia Wang (QIUHE)" <Qi...@microsoft.com>.
Just to clarify:
A PR starts with only one commit by contributor.
After final CR, the reviewer informs contributor to do final squash?
Thanks,
Julia
-----Original Message-----
From: Byung-Gon Chun [mailto:bgchun@gmail.com]
Sent: Sunday, January 25, 2015 4:31 PM
To: dev@reef.incubator.apache.org
Subject: Re: Pull request hygiene
+1
Let's reject PRs that do not meet the formal guidelines.
On Mon, Jan 26, 2015 at 4:32 AM, Markus Weimer <ma...@weimo.de> wrote:
> Hi,
>
> I don't think we should have the contributor do the final squash. It
> would mean that they have to create another PR, I believe. This makes
> bookkeeping much harder. If the committer does it, the code goes
> straight to Apache's git repository and can close the PR.
>
> In this thread, we seem to agree that the current split of work
> between contributor on the wiki is what we /want/ to do, yet we somehow don't.
> Shall we reject PRs on formal grounds then?
>
> Markus
>
>
> On 2015-01-24 00:29, John Yang wrote:
> > Great, that sounds good to me.
> >
> > John
> > On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)"
> > <Qi...@microsoft.com>
> > wrote:
> >
> >> I agree - having the PR start with only one commit - this is should
> >> be easy to achieve for contributor.
> >>
> >>
> >>
> >> After complete code review cycle and ready to merge, if necessary,
> >> committer can request contributor to do one more squash.
> >>
> >>
> >>
> >> -----Original Message-----
> >> From: Markus Weimer [mailto:markus@weimo.de]
> >> Sent: Friday, January 23, 2015 8:33 AM
> >> To: dev@reef.incubator.apache.org
> >> Subject: Re: Pull request hygiene
> >>
> >>
> >>
> >> Hi,
> >>
> >>
> >>
> >> it goes without saying that assigning blame was far from my intentions.
> >>
> >> There is just a certain amount of work that can be done either on
> >> the contributor or the committer side of the process. I understand
> >> that we
> all
> >> think that making the comitters' life easier is important. From
> >> that
> angle,
> >> having the contributor do the *final* squash isn't all that
> >> important,
> as
> >> the work there is limited.
> >>
> >>
> >>
> >> However, I find having the PR start with only one commit very helpful.
> >>
> >> It allows us to capture a good commit message right then, which
> >> GitHub will use as the PR description. It always picks the first
> >> commit for
> that,
> >> hence the need to squash. And it also makes the editing of the
> >> final
> squash
> >> much, much easier.
> >>
> >>
> >>
> >> Regarding item 3 on my list: We all are in agreement on this one.
> >> Hence, we can be obnoxious then and reject PRs that have no JIRA
> >> reference :-)
> >>
> >>
> >>
> >> Markus
> >>
> >>
> >>
> >>
> >>
> >> On 2015-01-22 17:57, John Yang wrote:
> >>
> >>> Hi,
> >>
> >>>
> >>
> >>>
> >>
> >>> I didn’t adhere to 1 and 2. Sorry about that.
> >>
> >>>
> >>
> >>> I also think it is a good idea to notify the contributor once the
> >>> PR is
> >> good to merge, so that he/she does not forget.
> >>
> >>>
> >>
> >>> 3 should definitely be kept.
> >>
> >>>
> >>
> >>>
> >>
> >>> Thanks,
> >>
> >>> John
> >>
> >>>
> >>
> >>>
> >>
> >>>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com<mailto:
> >> bgchun@gmail.com>> wrote:
> >>
> >>>>
> >>
> >>>> The option sounds good to me.
> >>
> >>>>
> >>
> >>>> ---
> >>
> >>>> Byung-Gon Chun
> >>
> >>>>
> >>
> >>>> Sent from my phone
> >>
> >>>>
> >>
> >>>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qiuhe.Wang@microsoft.com
> >> <ma...@microsoft.com>> 작성:
> >>
> >>>>
> >>
> >>>>> We should definitely keep 3 as it gives us a document to trace
> >>>>> why we
> >> made the change.
> >>
> >>>>>
> >>
> >>>>> 1 is little bit hard. Even if we squash before making a new pull
> >> request, after receiving comments and we make some changes, would
> >> you
> like
> >> to keep the delta or you would like to squash them into one commit?
> >> And
> as
> >> we could back force a few times during the code review, or
> >> sometimes may just make very minor format change, squash each time
> >> seems quite troublesome.
> >>
> >>>>>
> >>
> >>>>> One option is when the code is ready to merge, the committer and
> >> inform the contributor to do onetime squash. The contributor can
> >> use
> this
> >> opportunity to put proper comments as it can be changed during the
> >> code review as well.
> >>
> >>>>>
> >>
> >>>>> Thanks,
> >>
> >>>>> Julia
> >>
> >>>>>
> >>
> >>>>> -----Original Message-----
> >>
> >>>>> From: Markus Weimer [mailto:markus@weimo.de]
> >>
> >>>>> Sent: Thursday, January 22, 2015 3:53 PM
> >>
> >>>>> To: REEF Developers Mailinglist
> >>
> >>>>> Subject: Pull request hygiene
> >>
> >>>>>
> >>
> >>>>> Hi,
> >>
> >>>>>
> >>
> >>>>> I have been merging several pull requests recently that did not
> adhere
> >> to the guidelines we gave ourselves. Most notably, the following
> >> things didn't happen:
> >>
> >>>>>
> >>
> >>>>> 1. The PR was not squashed to a single commit when it was opened.
> >>
> >>>>> 2. The one commit didn't have a message in accordance with our
> >>
> >>>>> contributor's guidelines.
> >>
> >>>>> 3. The PR wasn't announced on JIRA.
> >>
> >>>>>
> >>
> >>>>> Given that we don't follow our own rules, I'd like to know why
> >>>>> and
> >> what we want to do about it?
> >>
> >>>>>
> >>
> >>>>> For instance We could drop (1) and (2) in the above list, which
> >>>>> would
> >> just move work from the contributor to the committer. This is
> >> exactly
> what
> >> happened for the aforementioned PRs.
> >>
> >>>>>
> >>
> >>>>> Number 3 above seems crucial to me. If we have PRs that
> >>>>> completely
> >> sideline JIRA, we risk loosing community consensus along the way.
> >>
> >>>>>
> >>
> >>>>> Thoughts? Comments?
> >>
> >>>>>
> >>
> >>>>> Markus
> >>
> >>>
> >>
> >
>
--
Byung-Gon Chun
Re: Pull request hygiene
Posted by Byung-Gon Chun <bg...@gmail.com>.
+1
Let's reject PRs that do not meet the formal guidelines.
On Mon, Jan 26, 2015 at 4:32 AM, Markus Weimer <ma...@weimo.de> wrote:
> Hi,
>
> I don't think we should have the contributor do the final squash. It
> would mean that they have to create another PR, I believe. This makes
> bookkeeping much harder. If the committer does it, the code goes
> straight to Apache's git repository and can close the PR.
>
> In this thread, we seem to agree that the current split of work between
> contributor on the wiki is what we /want/ to do, yet we somehow don't.
> Shall we reject PRs on formal grounds then?
>
> Markus
>
>
> On 2015-01-24 00:29, John Yang wrote:
> > Great, that sounds good to me.
> >
> > John
> > On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)" <Qi...@microsoft.com>
> > wrote:
> >
> >> I agree - having the PR start with only one commit - this is should be
> >> easy to achieve for contributor.
> >>
> >>
> >>
> >> After complete code review cycle and ready to merge, if necessary,
> >> committer can request contributor to do one more squash.
> >>
> >>
> >>
> >> -----Original Message-----
> >> From: Markus Weimer [mailto:markus@weimo.de]
> >> Sent: Friday, January 23, 2015 8:33 AM
> >> To: dev@reef.incubator.apache.org
> >> Subject: Re: Pull request hygiene
> >>
> >>
> >>
> >> Hi,
> >>
> >>
> >>
> >> it goes without saying that assigning blame was far from my intentions.
> >>
> >> There is just a certain amount of work that can be done either on the
> >> contributor or the committer side of the process. I understand that we
> all
> >> think that making the comitters' life easier is important. From that
> angle,
> >> having the contributor do the *final* squash isn't all that important,
> as
> >> the work there is limited.
> >>
> >>
> >>
> >> However, I find having the PR start with only one commit very helpful.
> >>
> >> It allows us to capture a good commit message right then, which GitHub
> >> will use as the PR description. It always picks the first commit for
> that,
> >> hence the need to squash. And it also makes the editing of the final
> squash
> >> much, much easier.
> >>
> >>
> >>
> >> Regarding item 3 on my list: We all are in agreement on this one. Hence,
> >> we can be obnoxious then and reject PRs that have no JIRA reference :-)
> >>
> >>
> >>
> >> Markus
> >>
> >>
> >>
> >>
> >>
> >> On 2015-01-22 17:57, John Yang wrote:
> >>
> >>> Hi,
> >>
> >>>
> >>
> >>>
> >>
> >>> I didn’t adhere to 1 and 2. Sorry about that.
> >>
> >>>
> >>
> >>> I also think it is a good idea to notify the contributor once the PR is
> >> good to merge, so that he/she does not forget.
> >>
> >>>
> >>
> >>> 3 should definitely be kept.
> >>
> >>>
> >>
> >>>
> >>
> >>> Thanks,
> >>
> >>> John
> >>
> >>>
> >>
> >>>
> >>
> >>>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com<mailto:
> >> bgchun@gmail.com>> wrote:
> >>
> >>>>
> >>
> >>>> The option sounds good to me.
> >>
> >>>>
> >>
> >>>> ---
> >>
> >>>> Byung-Gon Chun
> >>
> >>>>
> >>
> >>>> Sent from my phone
> >>
> >>>>
> >>
> >>>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qiuhe.Wang@microsoft.com
> >> <ma...@microsoft.com>> 작성:
> >>
> >>>>
> >>
> >>>>> We should definitely keep 3 as it gives us a document to trace why we
> >> made the change.
> >>
> >>>>>
> >>
> >>>>> 1 is little bit hard. Even if we squash before making a new pull
> >> request, after receiving comments and we make some changes, would you
> like
> >> to keep the delta or you would like to squash them into one commit? And
> as
> >> we could back force a few times during the code review, or sometimes may
> >> just make very minor format change, squash each time seems quite
> >> troublesome.
> >>
> >>>>>
> >>
> >>>>> One option is when the code is ready to merge, the committer and
> >> inform the contributor to do onetime squash. The contributor can use
> this
> >> opportunity to put proper comments as it can be changed during the code
> >> review as well.
> >>
> >>>>>
> >>
> >>>>> Thanks,
> >>
> >>>>> Julia
> >>
> >>>>>
> >>
> >>>>> -----Original Message-----
> >>
> >>>>> From: Markus Weimer [mailto:markus@weimo.de]
> >>
> >>>>> Sent: Thursday, January 22, 2015 3:53 PM
> >>
> >>>>> To: REEF Developers Mailinglist
> >>
> >>>>> Subject: Pull request hygiene
> >>
> >>>>>
> >>
> >>>>> Hi,
> >>
> >>>>>
> >>
> >>>>> I have been merging several pull requests recently that did not
> adhere
> >> to the guidelines we gave ourselves. Most notably, the following things
> >> didn't happen:
> >>
> >>>>>
> >>
> >>>>> 1. The PR was not squashed to a single commit when it was opened.
> >>
> >>>>> 2. The one commit didn't have a message in accordance with our
> >>
> >>>>> contributor's guidelines.
> >>
> >>>>> 3. The PR wasn't announced on JIRA.
> >>
> >>>>>
> >>
> >>>>> Given that we don't follow our own rules, I'd like to know why and
> >> what we want to do about it?
> >>
> >>>>>
> >>
> >>>>> For instance We could drop (1) and (2) in the above list, which would
> >> just move work from the contributor to the committer. This is exactly
> what
> >> happened for the aforementioned PRs.
> >>
> >>>>>
> >>
> >>>>> Number 3 above seems crucial to me. If we have PRs that completely
> >> sideline JIRA, we risk loosing community consensus along the way.
> >>
> >>>>>
> >>
> >>>>> Thoughts? Comments?
> >>
> >>>>>
> >>
> >>>>> Markus
> >>
> >>>
> >>
> >
>
--
Byung-Gon Chun
Re: Pull request hygiene
Posted by Markus Weimer <ma...@weimo.de>.
Hi,
I don't think we should have the contributor do the final squash. It
would mean that they have to create another PR, I believe. This makes
bookkeeping much harder. If the committer does it, the code goes
straight to Apache's git repository and can close the PR.
In this thread, we seem to agree that the current split of work between
contributor on the wiki is what we /want/ to do, yet we somehow don't.
Shall we reject PRs on formal grounds then?
Markus
On 2015-01-24 00:29, John Yang wrote:
> Great, that sounds good to me.
>
> John
> On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)" <Qi...@microsoft.com>
> wrote:
>
>> I agree - having the PR start with only one commit - this is should be
>> easy to achieve for contributor.
>>
>>
>>
>> After complete code review cycle and ready to merge, if necessary,
>> committer can request contributor to do one more squash.
>>
>>
>>
>> -----Original Message-----
>> From: Markus Weimer [mailto:markus@weimo.de]
>> Sent: Friday, January 23, 2015 8:33 AM
>> To: dev@reef.incubator.apache.org
>> Subject: Re: Pull request hygiene
>>
>>
>>
>> Hi,
>>
>>
>>
>> it goes without saying that assigning blame was far from my intentions.
>>
>> There is just a certain amount of work that can be done either on the
>> contributor or the committer side of the process. I understand that we all
>> think that making the comitters' life easier is important. From that angle,
>> having the contributor do the *final* squash isn't all that important, as
>> the work there is limited.
>>
>>
>>
>> However, I find having the PR start with only one commit very helpful.
>>
>> It allows us to capture a good commit message right then, which GitHub
>> will use as the PR description. It always picks the first commit for that,
>> hence the need to squash. And it also makes the editing of the final squash
>> much, much easier.
>>
>>
>>
>> Regarding item 3 on my list: We all are in agreement on this one. Hence,
>> we can be obnoxious then and reject PRs that have no JIRA reference :-)
>>
>>
>>
>> Markus
>>
>>
>>
>>
>>
>> On 2015-01-22 17:57, John Yang wrote:
>>
>>> Hi,
>>
>>>
>>
>>>
>>
>>> I didn’t adhere to 1 and 2. Sorry about that.
>>
>>>
>>
>>> I also think it is a good idea to notify the contributor once the PR is
>> good to merge, so that he/she does not forget.
>>
>>>
>>
>>> 3 should definitely be kept.
>>
>>>
>>
>>>
>>
>>> Thanks,
>>
>>> John
>>
>>>
>>
>>>
>>
>>>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com<mailto:
>> bgchun@gmail.com>> wrote:
>>
>>>>
>>
>>>> The option sounds good to me.
>>
>>>>
>>
>>>> ---
>>
>>>> Byung-Gon Chun
>>
>>>>
>>
>>>> Sent from my phone
>>
>>>>
>>
>>>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qiuhe.Wang@microsoft.com
>> <ma...@microsoft.com>> 작성:
>>
>>>>
>>
>>>>> We should definitely keep 3 as it gives us a document to trace why we
>> made the change.
>>
>>>>>
>>
>>>>> 1 is little bit hard. Even if we squash before making a new pull
>> request, after receiving comments and we make some changes, would you like
>> to keep the delta or you would like to squash them into one commit? And as
>> we could back force a few times during the code review, or sometimes may
>> just make very minor format change, squash each time seems quite
>> troublesome.
>>
>>>>>
>>
>>>>> One option is when the code is ready to merge, the committer and
>> inform the contributor to do onetime squash. The contributor can use this
>> opportunity to put proper comments as it can be changed during the code
>> review as well.
>>
>>>>>
>>
>>>>> Thanks,
>>
>>>>> Julia
>>
>>>>>
>>
>>>>> -----Original Message-----
>>
>>>>> From: Markus Weimer [mailto:markus@weimo.de]
>>
>>>>> Sent: Thursday, January 22, 2015 3:53 PM
>>
>>>>> To: REEF Developers Mailinglist
>>
>>>>> Subject: Pull request hygiene
>>
>>>>>
>>
>>>>> Hi,
>>
>>>>>
>>
>>>>> I have been merging several pull requests recently that did not adhere
>> to the guidelines we gave ourselves. Most notably, the following things
>> didn't happen:
>>
>>>>>
>>
>>>>> 1. The PR was not squashed to a single commit when it was opened.
>>
>>>>> 2. The one commit didn't have a message in accordance with our
>>
>>>>> contributor's guidelines.
>>
>>>>> 3. The PR wasn't announced on JIRA.
>>
>>>>>
>>
>>>>> Given that we don't follow our own rules, I'd like to know why and
>> what we want to do about it?
>>
>>>>>
>>
>>>>> For instance We could drop (1) and (2) in the above list, which would
>> just move work from the contributor to the committer. This is exactly what
>> happened for the aforementioned PRs.
>>
>>>>>
>>
>>>>> Number 3 above seems crucial to me. If we have PRs that completely
>> sideline JIRA, we risk loosing community consensus along the way.
>>
>>>>>
>>
>>>>> Thoughts? Comments?
>>
>>>>>
>>
>>>>> Markus
>>
>>>
>>
>
RE: Pull request hygiene
Posted by John Yang <jo...@gmail.com>.
Great, that sounds good to me.
John
On Jan 24, 2015 2:44 AM, "Julia Wang (QIUHE)" <Qi...@microsoft.com>
wrote:
> I agree - having the PR start with only one commit - this is should be
> easy to achieve for contributor.
>
>
>
> After complete code review cycle and ready to merge, if necessary,
> committer can request contributor to do one more squash.
>
>
>
> -----Original Message-----
> From: Markus Weimer [mailto:markus@weimo.de]
> Sent: Friday, January 23, 2015 8:33 AM
> To: dev@reef.incubator.apache.org
> Subject: Re: Pull request hygiene
>
>
>
> Hi,
>
>
>
> it goes without saying that assigning blame was far from my intentions.
>
> There is just a certain amount of work that can be done either on the
> contributor or the committer side of the process. I understand that we all
> think that making the comitters' life easier is important. From that angle,
> having the contributor do the *final* squash isn't all that important, as
> the work there is limited.
>
>
>
> However, I find having the PR start with only one commit very helpful.
>
> It allows us to capture a good commit message right then, which GitHub
> will use as the PR description. It always picks the first commit for that,
> hence the need to squash. And it also makes the editing of the final squash
> much, much easier.
>
>
>
> Regarding item 3 on my list: We all are in agreement on this one. Hence,
> we can be obnoxious then and reject PRs that have no JIRA reference :-)
>
>
>
> Markus
>
>
>
>
>
> On 2015-01-22 17:57, John Yang wrote:
>
> > Hi,
>
> >
>
> >
>
> > I didn’t adhere to 1 and 2. Sorry about that.
>
> >
>
> > I also think it is a good idea to notify the contributor once the PR is
> good to merge, so that he/she does not forget.
>
> >
>
> > 3 should definitely be kept.
>
> >
>
> >
>
> > Thanks,
>
> > John
>
> >
>
> >
>
> >> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bgchun@gmail.com<mailto:
> bgchun@gmail.com>> wrote:
>
> >>
>
> >> The option sounds good to me.
>
> >>
>
> >> ---
>
> >> Byung-Gon Chun
>
> >>
>
> >> Sent from my phone
>
> >>
>
> >> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qiuhe.Wang@microsoft.com
> <ma...@microsoft.com>> 작성:
>
> >>
>
> >>> We should definitely keep 3 as it gives us a document to trace why we
> made the change.
>
> >>>
>
> >>> 1 is little bit hard. Even if we squash before making a new pull
> request, after receiving comments and we make some changes, would you like
> to keep the delta or you would like to squash them into one commit? And as
> we could back force a few times during the code review, or sometimes may
> just make very minor format change, squash each time seems quite
> troublesome.
>
> >>>
>
> >>> One option is when the code is ready to merge, the committer and
> inform the contributor to do onetime squash. The contributor can use this
> opportunity to put proper comments as it can be changed during the code
> review as well.
>
> >>>
>
> >>> Thanks,
>
> >>> Julia
>
> >>>
>
> >>> -----Original Message-----
>
> >>> From: Markus Weimer [mailto:markus@weimo.de]
>
> >>> Sent: Thursday, January 22, 2015 3:53 PM
>
> >>> To: REEF Developers Mailinglist
>
> >>> Subject: Pull request hygiene
>
> >>>
>
> >>> Hi,
>
> >>>
>
> >>> I have been merging several pull requests recently that did not adhere
> to the guidelines we gave ourselves. Most notably, the following things
> didn't happen:
>
> >>>
>
> >>> 1. The PR was not squashed to a single commit when it was opened.
>
> >>> 2. The one commit didn't have a message in accordance with our
>
> >>> contributor's guidelines.
>
> >>> 3. The PR wasn't announced on JIRA.
>
> >>>
>
> >>> Given that we don't follow our own rules, I'd like to know why and
> what we want to do about it?
>
> >>>
>
> >>> For instance We could drop (1) and (2) in the above list, which would
> just move work from the contributor to the committer. This is exactly what
> happened for the aforementioned PRs.
>
> >>>
>
> >>> Number 3 above seems crucial to me. If we have PRs that completely
> sideline JIRA, we risk loosing community consensus along the way.
>
> >>>
>
> >>> Thoughts? Comments?
>
> >>>
>
> >>> Markus
>
> >
>
RE: Pull request hygiene
Posted by "Julia Wang (QIUHE)" <Qi...@microsoft.com>.
I agree - having the PR start with only one commit - this is should be easy to achieve for contributor.
After complete code review cycle and ready to merge, if necessary, committer can request contributor to do one more squash.
-----Original Message-----
From: Markus Weimer [mailto:markus@weimo.de]
Sent: Friday, January 23, 2015 8:33 AM
To: dev@reef.incubator.apache.org
Subject: Re: Pull request hygiene
Hi,
it goes without saying that assigning blame was far from my intentions.
There is just a certain amount of work that can be done either on the contributor or the committer side of the process. I understand that we all think that making the comitters' life easier is important. From that angle, having the contributor do the *final* squash isn't all that important, as the work there is limited.
However, I find having the PR start with only one commit very helpful.
It allows us to capture a good commit message right then, which GitHub will use as the PR description. It always picks the first commit for that, hence the need to squash. And it also makes the editing of the final squash much, much easier.
Regarding item 3 on my list: We all are in agreement on this one. Hence, we can be obnoxious then and reject PRs that have no JIRA reference :-)
Markus
On 2015-01-22 17:57, John Yang wrote:
> Hi,
>
>
> I didn’t adhere to 1 and 2. Sorry about that.
>
> I also think it is a good idea to notify the contributor once the PR is good to merge, so that he/she does not forget.
>
> 3 should definitely be kept.
>
>
> Thanks,
> John
>
>
>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bg...@gmail.com>> wrote:
>>
>> The option sounds good to me.
>>
>> ---
>> Byung-Gon Chun
>>
>> Sent from my phone
>>
>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qi...@microsoft.com>> 작성:
>>
>>> We should definitely keep 3 as it gives us a document to trace why we made the change.
>>>
>>> 1 is little bit hard. Even if we squash before making a new pull request, after receiving comments and we make some changes, would you like to keep the delta or you would like to squash them into one commit? And as we could back force a few times during the code review, or sometimes may just make very minor format change, squash each time seems quite troublesome.
>>>
>>> One option is when the code is ready to merge, the committer and inform the contributor to do onetime squash. The contributor can use this opportunity to put proper comments as it can be changed during the code review as well.
>>>
>>> Thanks,
>>> Julia
>>>
>>> -----Original Message-----
>>> From: Markus Weimer [mailto:markus@weimo.de]
>>> Sent: Thursday, January 22, 2015 3:53 PM
>>> To: REEF Developers Mailinglist
>>> Subject: Pull request hygiene
>>>
>>> Hi,
>>>
>>> I have been merging several pull requests recently that did not adhere to the guidelines we gave ourselves. Most notably, the following things didn't happen:
>>>
>>> 1. The PR was not squashed to a single commit when it was opened.
>>> 2. The one commit didn't have a message in accordance with our
>>> contributor's guidelines.
>>> 3. The PR wasn't announced on JIRA.
>>>
>>> Given that we don't follow our own rules, I'd like to know why and what we want to do about it?
>>>
>>> For instance We could drop (1) and (2) in the above list, which would just move work from the contributor to the committer. This is exactly what happened for the aforementioned PRs.
>>>
>>> Number 3 above seems crucial to me. If we have PRs that completely sideline JIRA, we risk loosing community consensus along the way.
>>>
>>> Thoughts? Comments?
>>>
>>> Markus
>
Re: Pull request hygiene
Posted by Markus Weimer <ma...@weimo.de>.
Hi,
it goes without saying that assigning blame was far from my intentions.
There is just a certain amount of work that can be done either on the
contributor or the committer side of the process. I understand that we
all think that making the comitters' life easier is important. From that
angle, having the contributor do the *final* squash isn't all that
important, as the work there is limited.
However, I find having the PR start with only one commit very helpful.
It allows us to capture a good commit message right then, which GitHub
will use as the PR description. It always picks the first commit for
that, hence the need to squash. And it also makes the editing of the
final squash much, much easier.
Regarding item 3 on my list: We all are in agreement on this one. Hence,
we can be obnoxious then and reject PRs that have no JIRA reference :-)
Markus
On 2015-01-22 17:57, John Yang wrote:
> Hi,
>
>
> I didn’t adhere to 1 and 2. Sorry about that.
>
> I also think it is a good idea to notify the contributor once the PR is good to merge, so that he/she does not forget.
>
> 3 should definitely be kept.
>
>
> Thanks,
> John
>
>
>> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bg...@gmail.com> wrote:
>>
>> The option sounds good to me.
>>
>> ---
>> Byung-Gon Chun
>>
>> Sent from my phone
>>
>> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qi...@microsoft.com> 작성:
>>
>>> We should definitely keep 3 as it gives us a document to trace why we made the change.
>>>
>>> 1 is little bit hard. Even if we squash before making a new pull request, after receiving comments and we make some changes, would you like to keep the delta or you would like to squash them into one commit? And as we could back force a few times during the code review, or sometimes may just make very minor format change, squash each time seems quite troublesome.
>>>
>>> One option is when the code is ready to merge, the committer and inform the contributor to do onetime squash. The contributor can use this opportunity to put proper comments as it can be changed during the code review as well.
>>>
>>> Thanks,
>>> Julia
>>>
>>> -----Original Message-----
>>> From: Markus Weimer [mailto:markus@weimo.de]
>>> Sent: Thursday, January 22, 2015 3:53 PM
>>> To: REEF Developers Mailinglist
>>> Subject: Pull request hygiene
>>>
>>> Hi,
>>>
>>> I have been merging several pull requests recently that did not adhere to the guidelines we gave ourselves. Most notably, the following things didn't happen:
>>>
>>> 1. The PR was not squashed to a single commit when it was opened.
>>> 2. The one commit didn't have a message in accordance with our
>>> contributor's guidelines.
>>> 3. The PR wasn't announced on JIRA.
>>>
>>> Given that we don't follow our own rules, I'd like to know why and what we want to do about it?
>>>
>>> For instance We could drop (1) and (2) in the above list, which would just move work from the contributor to the committer. This is exactly what happened for the aforementioned PRs.
>>>
>>> Number 3 above seems crucial to me. If we have PRs that completely sideline JIRA, we risk loosing community consensus along the way.
>>>
>>> Thoughts? Comments?
>>>
>>> Markus
>
Re: Pull request hygiene
Posted by John Yang <jo...@gmail.com>.
Hi,
I didn’t adhere to 1 and 2. Sorry about that.
I also think it is a good idea to notify the contributor once the PR is good to merge, so that he/she does not forget.
3 should definitely be kept.
Thanks,
John
> On Jan 23, 2015, at 9:51 AM, Byung-Gon Chun <bg...@gmail.com> wrote:
>
> The option sounds good to me.
>
> ---
> Byung-Gon Chun
>
> Sent from my phone
>
> 2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qi...@microsoft.com> 작성:
>
>> We should definitely keep 3 as it gives us a document to trace why we made the change.
>>
>> 1 is little bit hard. Even if we squash before making a new pull request, after receiving comments and we make some changes, would you like to keep the delta or you would like to squash them into one commit? And as we could back force a few times during the code review, or sometimes may just make very minor format change, squash each time seems quite troublesome.
>>
>> One option is when the code is ready to merge, the committer and inform the contributor to do onetime squash. The contributor can use this opportunity to put proper comments as it can be changed during the code review as well.
>>
>> Thanks,
>> Julia
>>
>> -----Original Message-----
>> From: Markus Weimer [mailto:markus@weimo.de]
>> Sent: Thursday, January 22, 2015 3:53 PM
>> To: REEF Developers Mailinglist
>> Subject: Pull request hygiene
>>
>> Hi,
>>
>> I have been merging several pull requests recently that did not adhere to the guidelines we gave ourselves. Most notably, the following things didn't happen:
>>
>> 1. The PR was not squashed to a single commit when it was opened.
>> 2. The one commit didn't have a message in accordance with our
>> contributor's guidelines.
>> 3. The PR wasn't announced on JIRA.
>>
>> Given that we don't follow our own rules, I'd like to know why and what we want to do about it?
>>
>> For instance We could drop (1) and (2) in the above list, which would just move work from the contributor to the committer. This is exactly what happened for the aforementioned PRs.
>>
>> Number 3 above seems crucial to me. If we have PRs that completely sideline JIRA, we risk loosing community consensus along the way.
>>
>> Thoughts? Comments?
>>
>> Markus
Re: Pull request hygiene
Posted by Byung-Gon Chun <bg...@gmail.com>.
The option sounds good to me.
---
Byung-Gon Chun
Sent from my phone
2015. 1. 23. 오전 9:04 Julia Wang (QIUHE) <Qi...@microsoft.com> 작성:
> We should definitely keep 3 as it gives us a document to trace why we made the change.
>
> 1 is little bit hard. Even if we squash before making a new pull request, after receiving comments and we make some changes, would you like to keep the delta or you would like to squash them into one commit? And as we could back force a few times during the code review, or sometimes may just make very minor format change, squash each time seems quite troublesome.
>
> One option is when the code is ready to merge, the committer and inform the contributor to do onetime squash. The contributor can use this opportunity to put proper comments as it can be changed during the code review as well.
>
> Thanks,
> Julia
>
> -----Original Message-----
> From: Markus Weimer [mailto:markus@weimo.de]
> Sent: Thursday, January 22, 2015 3:53 PM
> To: REEF Developers Mailinglist
> Subject: Pull request hygiene
>
> Hi,
>
> I have been merging several pull requests recently that did not adhere to the guidelines we gave ourselves. Most notably, the following things didn't happen:
>
> 1. The PR was not squashed to a single commit when it was opened.
> 2. The one commit didn't have a message in accordance with our
> contributor's guidelines.
> 3. The PR wasn't announced on JIRA.
>
> Given that we don't follow our own rules, I'd like to know why and what we want to do about it?
>
> For instance We could drop (1) and (2) in the above list, which would just move work from the contributor to the committer. This is exactly what happened for the aforementioned PRs.
>
> Number 3 above seems crucial to me. If we have PRs that completely sideline JIRA, we risk loosing community consensus along the way.
>
> Thoughts? Comments?
>
> Markus
RE: Pull request hygiene
Posted by "Julia Wang (QIUHE)" <Qi...@microsoft.com>.
We should definitely keep 3 as it gives us a document to trace why we made the change.
1 is little bit hard. Even if we squash before making a new pull request, after receiving comments and we make some changes, would you like to keep the delta or you would like to squash them into one commit? And as we could back force a few times during the code review, or sometimes may just make very minor format change, squash each time seems quite troublesome.
One option is when the code is ready to merge, the committer and inform the contributor to do onetime squash. The contributor can use this opportunity to put proper comments as it can be changed during the code review as well.
Thanks,
Julia
-----Original Message-----
From: Markus Weimer [mailto:markus@weimo.de]
Sent: Thursday, January 22, 2015 3:53 PM
To: REEF Developers Mailinglist
Subject: Pull request hygiene
Hi,
I have been merging several pull requests recently that did not adhere to the guidelines we gave ourselves. Most notably, the following things didn't happen:
1. The PR was not squashed to a single commit when it was opened.
2. The one commit didn't have a message in accordance with our
contributor's guidelines.
3. The PR wasn't announced on JIRA.
Given that we don't follow our own rules, I'd like to know why and what we want to do about it?
For instance We could drop (1) and (2) in the above list, which would just move work from the contributor to the committer. This is exactly what happened for the aforementioned PRs.
Number 3 above seems crucial to me. If we have PRs that completely sideline JIRA, we risk loosing community consensus along the way.
Thoughts? Comments?
Markus