From: Kaartic Sivaraam Date: Tue, 15 Aug 2017 09:31:14 GMT Subject: Re: [PATCH] hook: use correct logical variable Message-ID: <4e2c5bd8-48c9-fc8c-2c2c-ede3951019fc@gmail.com> In-Reply-To: On Monday 14 August 2017 11:24 PM, Stefan Beller wrote: > On Mon, Aug 14, 2017 at 1:46 AM, Kaartic Sivaraam > wrote: >> Sign-off added should be that of the "committer" not that of the >> "commit's author". >> >> Use the correct logical variable that identifies the committer. >> >> Signed-off-by: Kaartic Sivaraam >> --- >> This fixes a small issue when trying to do the following with the script enabled, >> >> $ git commit --amend -s >> >> If the commit being amended was signed off by the commit's author then the above command >> would *append* the sign-off of the committer followed by that of the commit's author. >> That' because the script is invoked only after the sign-off is added by the '-s' option AND >> the default of 'trailer.ifexists' for interpret-trailers currently defaults to the 'addIfDifferentNeighbor' >> thus interpret-trailer fails to identify the existing sign-off of the commit's author and adds it. > The background knowledge provided up to here seems like > a valuable information that we'd want to preserve in the commit > history, i.e. make it part of the commit message? I didn't do that previously expecting a few people would get confused by this (it did turn out to be true). I could have made it more clearer but didn't attempt as I thought it wasn't worth the effort. Yeah, it sometimes takes time to *simplify* things. I guess Junio's suggestion found below seems concise enough although it doesn't capture the reason I did the change. Sign-off added should be that of the "committer", not that of the "commit's author"; that is how the rest of Git adds sign-off using sequencer.c::append_signoff(). --- Kaartic