[REMINDER] Please signoff when committing other people's changes

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[REMINDER] Please signoff when committing other people's changes

Andre
dev,

May I remind you to ensure we follow the Contributor Guide and use:

git commit --amend -s

when merging commits from your peers?

While git pretty-format can be used to reveal the committer, I am sure that
all of us will agree that as an inclusive community we value both the
pretty and ugly formats...

So can we give the ugly format the support it deserves and ensure we add
the neat Signed-off-by stamp to the commit message?

Cheers
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Oleg Zhurakousky
Andre

Thanks for the reminder. I admit that I did not know that we require it in the Contributor Guide, so thanks for pointing it out.
However, your email did prompt me to look at the purpose and origin of the ‘-s’ flag and led me to this thread on Stack Overflow - http://stackoverflow.com/questions/1962094/what-is-the-sign-off-feature-in-git-for.

And I am now wondering if we should require it or even use it in the first place, since it’s origin, history and purpose appears to have more “individual” legal implications then showcasing the actual committer.

Thoughts?

Cheers
Oleg

On Mar 2, 2017, at 6:35 AM, Andre <[hidden email]<mailto:[hidden email]>> wrote:

dev,

May I remind you to ensure we follow the Contributor Guide and use:

git commit --amend -s

when merging commits from your peers?

While git pretty-format can be used to reveal the committer, I am sure that
all of us will agree that as an inclusive community we value both the
pretty and ugly formats...

So can we give the ugly format the support it deserves and ensure we add
the neat Signed-off-by stamp to the commit message?

Cheers

Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Bryan Bende
The sign-off is so we can easily see who the reviewer/merger was from
the git history.

We can always go back to the JIRA or PR and the reviewer/merger should
have commented there, but its convenient to see it in the git history
in my opinion.

Personally, whenever merging someones contribution I use "git am
--signoff < patchfile" which I guess is equivalent to doing the ammend
after applying the patch.


On Thu, Mar 2, 2017 at 8:05 AM, Oleg Zhurakousky
<[hidden email]> wrote:

> Andre
>
> Thanks for the reminder. I admit that I did not know that we require it in the Contributor Guide, so thanks for pointing it out.
> However, your email did prompt me to look at the purpose and origin of the ‘-s’ flag and led me to this thread on Stack Overflow - http://stackoverflow.com/questions/1962094/what-is-the-sign-off-feature-in-git-for.
>
> And I am now wondering if we should require it or even use it in the first place, since it’s origin, history and purpose appears to have more “individual” legal implications then showcasing the actual committer.
>
> Thoughts?
>
> Cheers
> Oleg
>
> On Mar 2, 2017, at 6:35 AM, Andre <[hidden email]<mailto:[hidden email]>> wrote:
>
> dev,
>
> May I remind you to ensure we follow the Contributor Guide and use:
>
> git commit --amend -s
>
> when merging commits from your peers?
>
> While git pretty-format can be used to reveal the committer, I am sure that
> all of us will agree that as an inclusive community we value both the
> pretty and ugly formats...
>
> So can we give the ugly format the support it deserves and ensure we add
> the neat Signed-off-by stamp to the commit message?
>
> Cheers
>
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Oleg Zhurakousky
Thanks Bryan.

If ‘-s’ is only for showcasing the committer I don’t believe anyone would have any issues with it, but my concern at the moment is purely legal, so I am not sure who is the right person to answer that, but figured raising the concern is the least I can do.

Cheers
Oleg


> On Mar 2, 2017, at 8:16 AM, Bryan Bende <[hidden email]> wrote:
>
> The sign-off is so we can easily see who the reviewer/merger was from
> the git history.
>
> We can always go back to the JIRA or PR and the reviewer/merger should
> have commented there, but its convenient to see it in the git history
> in my opinion.
>
> Personally, whenever merging someones contribution I use "git am
> --signoff < patchfile" which I guess is equivalent to doing the ammend
> after applying the patch.
>
>
> On Thu, Mar 2, 2017 at 8:05 AM, Oleg Zhurakousky
> <[hidden email]> wrote:
>> Andre
>>
>> Thanks for the reminder. I admit that I did not know that we require it in the Contributor Guide, so thanks for pointing it out.
>> However, your email did prompt me to look at the purpose and origin of the ‘-s’ flag and led me to this thread on Stack Overflow - http://stackoverflow.com/questions/1962094/what-is-the-sign-off-feature-in-git-for.
>>
>> And I am now wondering if we should require it or even use it in the first place, since it’s origin, history and purpose appears to have more “individual” legal implications then showcasing the actual committer.
>>
>> Thoughts?
>>
>> Cheers
>> Oleg
>>
>> On Mar 2, 2017, at 6:35 AM, Andre <[hidden email]<mailto:[hidden email]>> wrote:
>>
>> dev,
>>
>> May I remind you to ensure we follow the Contributor Guide and use:
>>
>> git commit --amend -s
>>
>> when merging commits from your peers?
>>
>> While git pretty-format can be used to reveal the committer, I am sure that
>> all of us will agree that as an inclusive community we value both the
>> pretty and ugly formats...
>>
>> So can we give the ugly format the support it deserves and ensure we add
>> the neat Signed-off-by stamp to the commit message?
>>
>> Cheers
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Matt Burgess-2
I didn't realize it was required either, I usually only sign off
(using the same thing Bryan Bende does) if the PR author couldn't
merge it on their own (i.e. not a NiFi committer/PMC). Certainly I can
start always signing off commits.

Regards,
Matt

On Thu, Mar 2, 2017 at 8:35 AM, Oleg Zhurakousky
<[hidden email]> wrote:

> Thanks Bryan.
>
> If ‘-s’ is only for showcasing the committer I don’t believe anyone would have any issues with it, but my concern at the moment is purely legal, so I am not sure who is the right person to answer that, but figured raising the concern is the least I can do.
>
> Cheers
> Oleg
>
>
>> On Mar 2, 2017, at 8:16 AM, Bryan Bende <[hidden email]> wrote:
>>
>> The sign-off is so we can easily see who the reviewer/merger was from
>> the git history.
>>
>> We can always go back to the JIRA or PR and the reviewer/merger should
>> have commented there, but its convenient to see it in the git history
>> in my opinion.
>>
>> Personally, whenever merging someones contribution I use "git am
>> --signoff < patchfile" which I guess is equivalent to doing the ammend
>> after applying the patch.
>>
>>
>> On Thu, Mar 2, 2017 at 8:05 AM, Oleg Zhurakousky
>> <[hidden email]> wrote:
>>> Andre
>>>
>>> Thanks for the reminder. I admit that I did not know that we require it in the Contributor Guide, so thanks for pointing it out.
>>> However, your email did prompt me to look at the purpose and origin of the ‘-s’ flag and led me to this thread on Stack Overflow - http://stackoverflow.com/questions/1962094/what-is-the-sign-off-feature-in-git-for.
>>>
>>> And I am now wondering if we should require it or even use it in the first place, since it’s origin, history and purpose appears to have more “individual” legal implications then showcasing the actual committer.
>>>
>>> Thoughts?
>>>
>>> Cheers
>>> Oleg
>>>
>>> On Mar 2, 2017, at 6:35 AM, Andre <[hidden email]<mailto:[hidden email]>> wrote:
>>>
>>> dev,
>>>
>>> May I remind you to ensure we follow the Contributor Guide and use:
>>>
>>> git commit --amend -s
>>>
>>> when merging commits from your peers?
>>>
>>> While git pretty-format can be used to reveal the committer, I am sure that
>>> all of us will agree that as an inclusive community we value both the
>>> pretty and ugly formats...
>>>
>>> So can we give the ugly format the support it deserves and ensure we add
>>> the neat Signed-off-by stamp to the commit message?
>>>
>>> Cheers
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Joe Witt
For what it is worth this is definitely not a requirement and not
something I knew anything of so I never do it.

I think it is a perfectly fine idea and a good practice to follow so
occasional reminders of its utility are fair game.  That said, to
Bryan's point I rely on the JIRA/issues history if i need to know who
did a given review.  So we have a couple of options.

But we should probably stop short of calling this a requirement.  In
an apache sense it is not.

Thanks
Joe

On Thu, Mar 2, 2017 at 8:51 AM, Matt Burgess <[hidden email]> wrote:

> I didn't realize it was required either, I usually only sign off
> (using the same thing Bryan Bende does) if the PR author couldn't
> merge it on their own (i.e. not a NiFi committer/PMC). Certainly I can
> start always signing off commits.
>
> Regards,
> Matt
>
> On Thu, Mar 2, 2017 at 8:35 AM, Oleg Zhurakousky
> <[hidden email]> wrote:
>> Thanks Bryan.
>>
>> If ‘-s’ is only for showcasing the committer I don’t believe anyone would have any issues with it, but my concern at the moment is purely legal, so I am not sure who is the right person to answer that, but figured raising the concern is the least I can do.
>>
>> Cheers
>> Oleg
>>
>>
>>> On Mar 2, 2017, at 8:16 AM, Bryan Bende <[hidden email]> wrote:
>>>
>>> The sign-off is so we can easily see who the reviewer/merger was from
>>> the git history.
>>>
>>> We can always go back to the JIRA or PR and the reviewer/merger should
>>> have commented there, but its convenient to see it in the git history
>>> in my opinion.
>>>
>>> Personally, whenever merging someones contribution I use "git am
>>> --signoff < patchfile" which I guess is equivalent to doing the ammend
>>> after applying the patch.
>>>
>>>
>>> On Thu, Mar 2, 2017 at 8:05 AM, Oleg Zhurakousky
>>> <[hidden email]> wrote:
>>>> Andre
>>>>
>>>> Thanks for the reminder. I admit that I did not know that we require it in the Contributor Guide, so thanks for pointing it out.
>>>> However, your email did prompt me to look at the purpose and origin of the ‘-s’ flag and led me to this thread on Stack Overflow - http://stackoverflow.com/questions/1962094/what-is-the-sign-off-feature-in-git-for.
>>>>
>>>> And I am now wondering if we should require it or even use it in the first place, since it’s origin, history and purpose appears to have more “individual” legal implications then showcasing the actual committer.
>>>>
>>>> Thoughts?
>>>>
>>>> Cheers
>>>> Oleg
>>>>
>>>> On Mar 2, 2017, at 6:35 AM, Andre <[hidden email]<mailto:[hidden email]>> wrote:
>>>>
>>>> dev,
>>>>
>>>> May I remind you to ensure we follow the Contributor Guide and use:
>>>>
>>>> git commit --amend -s
>>>>
>>>> when merging commits from your peers?
>>>>
>>>> While git pretty-format can be used to reveal the committer, I am sure that
>>>> all of us will agree that as an inclusive community we value both the
>>>> pretty and ugly formats...
>>>>
>>>> So can we give the ugly format the support it deserves and ensure we add
>>>> the neat Signed-off-by stamp to the commit message?
>>>>
>>>> Cheers
>>>>
>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Joe Skora
Like Andre, I originally got the requirement for signoff from the
Contributor Guide[1] when I started working on the project and later from
this email thread[2].  If this not the expected process, we should
definitely update the Contributor Guide.

From the Apache perspective the signoff confirms that the contribution was
reviewed for Apache compliance, but that's incumbent on anyone committing
to the repository.  Since the committer identity is available from the
repository the signoff seems redundant.

[1]https://cwiki.apache.org/confluence/display/NIFI/Contributor+Guide#
ContributorGuide-Stepstomerge/closepullrequestswithtwomainbranches
[2]
https://lists.apache.org/thread.html/1ce165a4172f67ce08683d3eb1c8253319a97dadd0ccc3fbc598f639@1446565288@%3Cdev.nifi.apache.org%3E


On Thu, Mar 2, 2017 at 9:33 AM, Joe Witt <[hidden email]> wrote:

> For what it is worth this is definitely not a requirement and not
> something I knew anything of so I never do it.
>
> I think it is a perfectly fine idea and a good practice to follow so
> occasional reminders of its utility are fair game.  That said, to
> Bryan's point I rely on the JIRA/issues history if i need to know who
> did a given review.  So we have a couple of options.
>
> But we should probably stop short of calling this a requirement.  In
> an apache sense it is not.
>
> Thanks
> Joe
>
> On Thu, Mar 2, 2017 at 8:51 AM, Matt Burgess <[hidden email]> wrote:
> > I didn't realize it was required either, I usually only sign off
> > (using the same thing Bryan Bende does) if the PR author couldn't
> > merge it on their own (i.e. not a NiFi committer/PMC). Certainly I can
> > start always signing off commits.
> >
> > Regards,
> > Matt
> >
> > On Thu, Mar 2, 2017 at 8:35 AM, Oleg Zhurakousky
> > <[hidden email]> wrote:
> >> Thanks Bryan.
> >>
> >> If ‘-s’ is only for showcasing the committer I don’t believe anyone
> would have any issues with it, but my concern at the moment is purely
> legal, so I am not sure who is the right person to answer that, but figured
> raising the concern is the least I can do.
> >>
> >> Cheers
> >> Oleg
> >>
> >>
> >>> On Mar 2, 2017, at 8:16 AM, Bryan Bende <[hidden email]> wrote:
> >>>
> >>> The sign-off is so we can easily see who the reviewer/merger was from
> >>> the git history.
> >>>
> >>> We can always go back to the JIRA or PR and the reviewer/merger should
> >>> have commented there, but its convenient to see it in the git history
> >>> in my opinion.
> >>>
> >>> Personally, whenever merging someones contribution I use "git am
> >>> --signoff < patchfile" which I guess is equivalent to doing the ammend
> >>> after applying the patch.
> >>>
> >>>
> >>> On Thu, Mar 2, 2017 at 8:05 AM, Oleg Zhurakousky
> >>> <[hidden email]> wrote:
> >>>> Andre
> >>>>
> >>>> Thanks for the reminder. I admit that I did not know that we require
> it in the Contributor Guide, so thanks for pointing it out.
> >>>> However, your email did prompt me to look at the purpose and origin
> of the ‘-s’ flag and led me to this thread on Stack Overflow -
> http://stackoverflow.com/questions/1962094/what-is-the-
> sign-off-feature-in-git-for.
> >>>>
> >>>> And I am now wondering if we should require it or even use it in the
> first place, since it’s origin, history and purpose appears to have more
> “individual” legal implications then showcasing the actual committer.
> >>>>
> >>>> Thoughts?
> >>>>
> >>>> Cheers
> >>>> Oleg
> >>>>
> >>>> On Mar 2, 2017, at 6:35 AM, Andre <[hidden email]<mailto:a
> [hidden email]>> wrote:
> >>>>
> >>>> dev,
> >>>>
> >>>> May I remind you to ensure we follow the Contributor Guide and use:
> >>>>
> >>>> git commit --amend -s
> >>>>
> >>>> when merging commits from your peers?
> >>>>
> >>>> While git pretty-format can be used to reveal the committer, I am
> sure that
> >>>> all of us will agree that as an inclusive community we value both the
> >>>> pretty and ugly formats...
> >>>>
> >>>> So can we give the ugly format the support it deserves and ensure we
> add
> >>>> the neat Signed-off-by stamp to the commit message?
> >>>>
> >>>> Cheers
> >>>>
> >>>
> >>
>
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Joe Witt
"If this not the expected process, we should definitely update the
Contributor Guide."

I think it is fine to encourage it.  It is not a requirement though.

The signoff is not an apache thing.  Committer privileges to push code
to a given repo is an apache thing.

We're an RTC community and the only thing we do require is a +1 from
another committer before the patch/PR is merged.  It is perfectly
sufficient for the reviewer to add a +1 on github comments or a JIRA
comment and the author could commit directly.  No signoff required.

Thanks
Joe

On Thu, Mar 2, 2017 at 10:42 AM, Joe Skora <[hidden email]> wrote:

> Like Andre, I originally got the requirement for signoff from the
> Contributor Guide[1] when I started working on the project and later from
> this email thread[2].  If this not the expected process, we should
> definitely update the Contributor Guide.
>
> From the Apache perspective the signoff confirms that the contribution was
> reviewed for Apache compliance, but that's incumbent on anyone committing
> to the repository.  Since the committer identity is available from the
> repository the signoff seems redundant.
>
> [1]https://cwiki.apache.org/confluence/display/NIFI/Contributor+Guide#
> ContributorGuide-Stepstomerge/closepullrequestswithtwomainbranches
> [2]
> https://lists.apache.org/thread.html/1ce165a4172f67ce08683d3eb1c8253319a97dadd0ccc3fbc598f639@1446565288@%3Cdev.nifi.apache.org%3E
>
>
> On Thu, Mar 2, 2017 at 9:33 AM, Joe Witt <[hidden email]> wrote:
>
>> For what it is worth this is definitely not a requirement and not
>> something I knew anything of so I never do it.
>>
>> I think it is a perfectly fine idea and a good practice to follow so
>> occasional reminders of its utility are fair game.  That said, to
>> Bryan's point I rely on the JIRA/issues history if i need to know who
>> did a given review.  So we have a couple of options.
>>
>> But we should probably stop short of calling this a requirement.  In
>> an apache sense it is not.
>>
>> Thanks
>> Joe
>>
>> On Thu, Mar 2, 2017 at 8:51 AM, Matt Burgess <[hidden email]> wrote:
>> > I didn't realize it was required either, I usually only sign off
>> > (using the same thing Bryan Bende does) if the PR author couldn't
>> > merge it on their own (i.e. not a NiFi committer/PMC). Certainly I can
>> > start always signing off commits.
>> >
>> > Regards,
>> > Matt
>> >
>> > On Thu, Mar 2, 2017 at 8:35 AM, Oleg Zhurakousky
>> > <[hidden email]> wrote:
>> >> Thanks Bryan.
>> >>
>> >> If ‘-s’ is only for showcasing the committer I don’t believe anyone
>> would have any issues with it, but my concern at the moment is purely
>> legal, so I am not sure who is the right person to answer that, but figured
>> raising the concern is the least I can do.
>> >>
>> >> Cheers
>> >> Oleg
>> >>
>> >>
>> >>> On Mar 2, 2017, at 8:16 AM, Bryan Bende <[hidden email]> wrote:
>> >>>
>> >>> The sign-off is so we can easily see who the reviewer/merger was from
>> >>> the git history.
>> >>>
>> >>> We can always go back to the JIRA or PR and the reviewer/merger should
>> >>> have commented there, but its convenient to see it in the git history
>> >>> in my opinion.
>> >>>
>> >>> Personally, whenever merging someones contribution I use "git am
>> >>> --signoff < patchfile" which I guess is equivalent to doing the ammend
>> >>> after applying the patch.
>> >>>
>> >>>
>> >>> On Thu, Mar 2, 2017 at 8:05 AM, Oleg Zhurakousky
>> >>> <[hidden email]> wrote:
>> >>>> Andre
>> >>>>
>> >>>> Thanks for the reminder. I admit that I did not know that we require
>> it in the Contributor Guide, so thanks for pointing it out.
>> >>>> However, your email did prompt me to look at the purpose and origin
>> of the ‘-s’ flag and led me to this thread on Stack Overflow -
>> http://stackoverflow.com/questions/1962094/what-is-the-
>> sign-off-feature-in-git-for.
>> >>>>
>> >>>> And I am now wondering if we should require it or even use it in the
>> first place, since it’s origin, history and purpose appears to have more
>> “individual” legal implications then showcasing the actual committer.
>> >>>>
>> >>>> Thoughts?
>> >>>>
>> >>>> Cheers
>> >>>> Oleg
>> >>>>
>> >>>> On Mar 2, 2017, at 6:35 AM, Andre <[hidden email]<mailto:a
>> [hidden email]>> wrote:
>> >>>>
>> >>>> dev,
>> >>>>
>> >>>> May I remind you to ensure we follow the Contributor Guide and use:
>> >>>>
>> >>>> git commit --amend -s
>> >>>>
>> >>>> when merging commits from your peers?
>> >>>>
>> >>>> While git pretty-format can be used to reveal the committer, I am
>> sure that
>> >>>> all of us will agree that as an inclusive community we value both the
>> >>>> pretty and ugly formats...
>> >>>>
>> >>>> So can we give the ugly format the support it deserves and ensure we
>> add
>> >>>> the neat Signed-off-by stamp to the commit message?
>> >>>>
>> >>>> Cheers
>> >>>>
>> >>>
>> >>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

James Wing
I recommend the practice.  Although the signoff may not be authoritative,
it requires a positive action that suggests you purposefully merged the
commit, as opposed to commits you might have accidentally merged and pushed.

Thanks,

James


On Thu, Mar 2, 2017 at 7:49 AM, Joe Witt <[hidden email]> wrote:

> "If this not the expected process, we should definitely update the
> Contributor Guide."
>
> I think it is fine to encourage it.  It is not a requirement though.
>
> The signoff is not an apache thing.  Committer privileges to push code
> to a given repo is an apache thing.
>
> We're an RTC community and the only thing we do require is a +1 from
> another committer before the patch/PR is merged.  It is perfectly
> sufficient for the reviewer to add a +1 on github comments or a JIRA
> comment and the author could commit directly.  No signoff required.
>
> Thanks
> Joe
>
> On Thu, Mar 2, 2017 at 10:42 AM, Joe Skora <[hidden email]> wrote:
> > Like Andre, I originally got the requirement for signoff from the
> > Contributor Guide[1] when I started working on the project and later from
> > this email thread[2].  If this not the expected process, we should
> > definitely update the Contributor Guide.
> >
> > From the Apache perspective the signoff confirms that the contribution
> was
> > reviewed for Apache compliance, but that's incumbent on anyone committing
> > to the repository.  Since the committer identity is available from the
> > repository the signoff seems redundant.
> >
> > [1]https://cwiki.apache.org/confluence/display/NIFI/Contributor+Guide#
> > ContributorGuide-Stepstomerge/closepullrequestswithtwomainbranches
> > [2]
> > https://lists.apache.org/thread.html/1ce165a4172f67ce08683d3eb1c825
> 3319a97dadd0ccc3fbc598f639@1446565288@%3Cdev.nifi.apache.org%3E
> >
> >
> > On Thu, Mar 2, 2017 at 9:33 AM, Joe Witt <[hidden email]> wrote:
> >
> >> For what it is worth this is definitely not a requirement and not
> >> something I knew anything of so I never do it.
> >>
> >> I think it is a perfectly fine idea and a good practice to follow so
> >> occasional reminders of its utility are fair game.  That said, to
> >> Bryan's point I rely on the JIRA/issues history if i need to know who
> >> did a given review.  So we have a couple of options.
> >>
> >> But we should probably stop short of calling this a requirement.  In
> >> an apache sense it is not.
> >>
> >> Thanks
> >> Joe
> >>
> >> On Thu, Mar 2, 2017 at 8:51 AM, Matt Burgess <[hidden email]>
> wrote:
> >> > I didn't realize it was required either, I usually only sign off
> >> > (using the same thing Bryan Bende does) if the PR author couldn't
> >> > merge it on their own (i.e. not a NiFi committer/PMC). Certainly I can
> >> > start always signing off commits.
> >> >
> >> > Regards,
> >> > Matt
> >> >
> >> > On Thu, Mar 2, 2017 at 8:35 AM, Oleg Zhurakousky
> >> > <[hidden email]> wrote:
> >> >> Thanks Bryan.
> >> >>
> >> >> If ‘-s’ is only for showcasing the committer I don’t believe anyone
> >> would have any issues with it, but my concern at the moment is purely
> >> legal, so I am not sure who is the right person to answer that, but
> figured
> >> raising the concern is the least I can do.
> >> >>
> >> >> Cheers
> >> >> Oleg
> >> >>
> >> >>
> >> >>> On Mar 2, 2017, at 8:16 AM, Bryan Bende <[hidden email]> wrote:
> >> >>>
> >> >>> The sign-off is so we can easily see who the reviewer/merger was
> from
> >> >>> the git history.
> >> >>>
> >> >>> We can always go back to the JIRA or PR and the reviewer/merger
> should
> >> >>> have commented there, but its convenient to see it in the git
> history
> >> >>> in my opinion.
> >> >>>
> >> >>> Personally, whenever merging someones contribution I use "git am
> >> >>> --signoff < patchfile" which I guess is equivalent to doing the
> ammend
> >> >>> after applying the patch.
> >> >>>
> >> >>>
> >> >>> On Thu, Mar 2, 2017 at 8:05 AM, Oleg Zhurakousky
> >> >>> <[hidden email]> wrote:
> >> >>>> Andre
> >> >>>>
> >> >>>> Thanks for the reminder. I admit that I did not know that we
> require
> >> it in the Contributor Guide, so thanks for pointing it out.
> >> >>>> However, your email did prompt me to look at the purpose and origin
> >> of the ‘-s’ flag and led me to this thread on Stack Overflow -
> >> http://stackoverflow.com/questions/1962094/what-is-the-
> >> sign-off-feature-in-git-for.
> >> >>>>
> >> >>>> And I am now wondering if we should require it or even use it in
> the
> >> first place, since it’s origin, history and purpose appears to have more
> >> “individual” legal implications then showcasing the actual committer.
> >> >>>>
> >> >>>> Thoughts?
> >> >>>>
> >> >>>> Cheers
> >> >>>> Oleg
> >> >>>>
> >> >>>> On Mar 2, 2017, at 6:35 AM, Andre <[hidden email]<mailto:a
> >> [hidden email]>> wrote:
> >> >>>>
> >> >>>> dev,
> >> >>>>
> >> >>>> May I remind you to ensure we follow the Contributor Guide and use:
> >> >>>>
> >> >>>> git commit --amend -s
> >> >>>>
> >> >>>> when merging commits from your peers?
> >> >>>>
> >> >>>> While git pretty-format can be used to reveal the committer, I am
> >> sure that
> >> >>>> all of us will agree that as an inclusive community we value both
> the
> >> >>>> pretty and ugly formats...
> >> >>>>
> >> >>>> So can we give the ugly format the support it deserves and ensure
> we
> >> add
> >> >>>> the neat Signed-off-by stamp to the commit message?
> >> >>>>
> >> >>>> Cheers
> >> >>>>
> >> >>>
> >> >>
> >>
>
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Andre
James,

There's no doubt the Sign-off-by is redundant (as GIT itself holds that
information, reason why GH is still able to show the information without
the sign-of-by stamp), however, I agree with your view around positive
action and easy to refer as Bryan pointed.

Joe,

Thanks for the clarification. If no-one opposes, I will update the
Contributor Guide regarding requirement vs. recommended as it seems to have
caused a small to confusion to some of the committers. If the message is
that consistency in this space is not required, than lets reflect this in
the documentation.

On a side note, it may be worth to note that a "+1 before merge" model
would sit in between CTR and RTC  - which technically seems to require
Consensus Approval (i.e. TTBOMK means 3 positive votes + lack of negative
votes in ASF lingo).

Formality of number aside, there's no doubt our model is working like a
charm! :-)

Cheers

On Fri, Mar 3, 2017 at 5:49 AM, James Wing <[hidden email]> wrote:

> I recommend the practice.  Although the signoff may not be authoritative,
> it requires a positive action that suggests you purposefully merged the
> commit, as opposed to commits you might have accidentally merged and
> pushed.
>
> Thanks,
>
> James
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Joe Witt
We are following rtc and specifically the form of rtc we adopted long ago
for commits to Nifi.  It simply requires a +1 before a merge and no
apparent lack of consensus.  Even then should there be disagreement after
the fact there are procedures to resolve.  Consensus forming is central to
the apache way.  Consensus forming in most cases and in healthy communities
can be achieved on most matters without formal votes.

What is generally not a good practice is open jira, add PR, get +1, merge
all in such a tight time window that folks could not reasonably discuss or
review.  So you'll notice that at least a 24 hour window should pass on
prs.  There are some truly trivial cases where this isn't really necessary
and fortunately no matter what we can always resolve issues even after a
commit.

I definitely am a fan of the model we have centered in on.

Thanks
Joe

On Mar 2, 2017 7:00 PM, "Andre" <[hidden email]> wrote:

> James,
>
> There's no doubt the Sign-off-by is redundant (as GIT itself holds that
> information, reason why GH is still able to show the information without
> the sign-of-by stamp), however, I agree with your view around positive
> action and easy to refer as Bryan pointed.
>
> Joe,
>
> Thanks for the clarification. If no-one opposes, I will update the
> Contributor Guide regarding requirement vs. recommended as it seems to have
> caused a small to confusion to some of the committers. If the message is
> that consistency in this space is not required, than lets reflect this in
> the documentation.
>
> On a side note, it may be worth to note that a "+1 before merge" model
> would sit in between CTR and RTC  - which technically seems to require
> Consensus Approval (i.e. TTBOMK means 3 positive votes + lack of negative
> votes in ASF lingo).
>
> Formality of number aside, there's no doubt our model is working like a
> charm! :-)
>
> Cheers
>
> On Fri, Mar 3, 2017 at 5:49 AM, James Wing <[hidden email]> wrote:
>
> > I recommend the practice.  Although the signoff may not be authoritative,
> > it requires a positive action that suggests you purposefully merged the
> > commit, as opposed to commits you might have accidentally merged and
> > pushed.
> >
> > Thanks,
> >
> > James
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Uwe@Moosheimer.com
+1

Am 03.03.2017 um 02:13 schrieb Joe Witt:

> We are following rtc and specifically the form of rtc we adopted long ago
> for commits to Nifi.  It simply requires a +1 before a merge and no
> apparent lack of consensus.  Even then should there be disagreement after
> the fact there are procedures to resolve.  Consensus forming is central to
> the apache way.  Consensus forming in most cases and in healthy communities
> can be achieved on most matters without formal votes.
>
> What is generally not a good practice is open jira, add PR, get +1, merge
> all in such a tight time window that folks could not reasonably discuss or
> review.  So you'll notice that at least a 24 hour window should pass on
> prs.  There are some truly trivial cases where this isn't really necessary
> and fortunately no matter what we can always resolve issues even after a
> commit.
>
> I definitely am a fan of the model we have centered in on.
>
> Thanks
> Joe
>
> On Mar 2, 2017 7:00 PM, "Andre" <[hidden email]> wrote:
>
>> James,
>>
>> There's no doubt the Sign-off-by is redundant (as GIT itself holds that
>> information, reason why GH is still able to show the information without
>> the sign-of-by stamp), however, I agree with your view around positive
>> action and easy to refer as Bryan pointed.
>>
>> Joe,
>>
>> Thanks for the clarification. If no-one opposes, I will update the
>> Contributor Guide regarding requirement vs. recommended as it seems to have
>> caused a small to confusion to some of the committers. If the message is
>> that consistency in this space is not required, than lets reflect this in
>> the documentation.
>>
>> On a side note, it may be worth to note that a "+1 before merge" model
>> would sit in between CTR and RTC  - which technically seems to require
>> Consensus Approval (i.e. TTBOMK means 3 positive votes + lack of negative
>> votes in ASF lingo).
>>
>> Formality of number aside, there's no doubt our model is working like a
>> charm! :-)
>>
>> Cheers
>>
>> On Fri, Mar 3, 2017 at 5:49 AM, James Wing <[hidden email]> wrote:
>>
>>> I recommend the practice.  Although the signoff may not be authoritative,
>>> it requires a positive action that suggests you purposefully merged the
>>> commit, as opposed to commits you might have accidentally merged and
>>> pushed.
>>>
>>> Thanks,
>>>
>>> James
>>>
>>>

Reply | Threaded
Open this post in threaded view
|

Re: [REMINDER] Please signoff when committing other people's changes

Sean Busbey-2
In reply to this post by Andre
On Thu, Mar 2, 2017 at 6:00 PM, Andre <[hidden email]> wrote:
> James,
>
> There's no doubt the Sign-off-by is redundant (as GIT itself holds that
> information, reason why GH is still able to show the information without
> the sign-of-by stamp), however, I agree with your view around positive
> action and easy to refer as Bryan pointed.
>


One important case where the signed-off-by isn't redundant (at least
within git) is when there is more than one reviewer. This has been
useful in some other projects, especially if folks get into the habit
and there are multiple places reviews happen (e.g. both on JIRA and
GitHub PRs). It's also been super helpful when non-committers are
doing reviews.

Another is when we get to the point of maintaining enough release
lines that the person doing a backport commit might not be the person
who did the initial review. The signed-off-by still carries useful
information (albeit something we can kind-of get from JIRA) about who
all did reviews.