Re: [mpich-devel] [mpich-discuss] Use of Signed-off-by in MPICH
In our workflow, often the commit is pushed to an ibm review branch, then the signer (usually me) does a signed-off cherry-pick to an anl review branch and pushes the code to mpich-ibm git repository for ANL to integrate. Using "signed off by" is usually much easier than playing git games to ensure that the author and committer are different. However, it really doesn't matter to IBM. We would prefer to follow a documented process that is used by ANL mpich team, but as of now I am not aware of any such documentation. The goal from our perspective is to ensure that an internal code review has been done before passing the code of to ANL - and it would be nice to know who at ANL reviewed the code before it was pushed to master. Michael Blocksome Parallel Environment MPI Middleware Team Lead, TCEM POWER, x86, and Blue Gene HPC Messaging [email protected] From: Jed Brown <[email protected]> To: Michael Blocksome/Rochester/IBM@IBMUS, Cc: [email protected] Date: 01/06/2014 01:21 PM Subject: Re: [mpich-discuss] Use of Signed-off-by in MPICH Michael Blocksome <[email protected]> writes:
IBM uses the "signed off by" feature to enforce/track code reviews before any code gets contributed to ANL. The intent is that the commit author != the commit signer when the code goes out. Often a developer at ANL will also review the code before it gets integrated into the master branch (IBM developers don't have push authority) and sign the commit again.
So the last stage (whoever makes the actual commit) is already available because Git has separate fields for Committer and Author, as shown by "git log --format=fuller".
We use the text "Signed-off-by:xxxx <yyy at zzz>" simply because this is what is supported by the git tools. You can use commands like `git cherry-pick -s`, `git commit --amend -s`, and `git am -s` to ease the signing effort.
Yes, though this only adds your own name. Do you email patches around until then? When I started this thread, I pointed out a recent case where the author and committer were the same, but someone else was providing a Signed-off-by. Either the commit went back and forth or (more likely) the committer amended the commit to add a Signed-off-by
From the reviewer. The idea with Git's -s is that you only ever sign for yourself, though if you integrate out-of-band content, you may copy the Signed-off-by tag provided by that other person. So the design of the -s option is not a perfect match, but I can see that it's often convenient. In contrast, Reviewed-by, Acked-by, and similar are almost always a result of the author or committer translating out-of-band information (e.g., an email discussion).
If you want to automatically add Reviewed-by tags, you could use .git/hooks/prepare-commit-msg, for example. Of course you can keep using Signed-off-by in place of Reviewed-by, though it causes some confusion for people new to the project, attempting to track provenance when either extracting code or using submodules with other projects that use the usual conventions, or scripts providing repository analytics.
participants (1)
-
Michael Blocksome