"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 <jsk...@gmail.com> 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 <joe.w...@gmail.com> 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 <mattyb...@apache.org> 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 >> > <ozhurakou...@hortonworks.com> 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 <bbe...@gmail.com> 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 >> >>> <ozhurakou...@hortonworks.com> 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 <andre-li...@fucs.org<mailto:a >> ndre-li...@fucs.org>> 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 >> >>>> >> >>> >> >> >>
