Re: [PATCH] hook: use correct logical variable

From: Stefan Beller <sbeller@google.com>
Date: Aug 14, 2017, 17:54 UTC
Message-ID: <CAGZ79kYPE1qQT6Lqd4M=uBUjX3e3p-ABOEm+ggAJMgfw0ENB0Q@mail.gmail.com>
In-Reply-To: <20170814084646.30781-1-kaarticsivaraam91196@gmail.com>

On Mon, Aug 14, 2017 at 1:46 AM, Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> wrote:

Show 16 quoted lines

> 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 <kaarticsivaraam91196@gmail.com>
> ---
>  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?

Code looks good.

Thanks, Stefan

Show 23 quoted lines

>
>  Anyways, it doesn't make sense for a script to add the sign-off of the commit's author. So,
>  fixing it seemed correct to me.
>
>  templates/hooks--prepare-commit-msg.sample | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index a84c3e5a8..12dd8fd88 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -34,7 +34,7 @@ SHA1=$3
>  #  *) ;;
>  # esac
>
> -# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
> +# SOB=$(git var GIT_COMMITTER_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
>  # git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
>  # if test -z "$COMMIT_SOURCE"
>  # then
> --
> 2.14.1.534.g641031ecb
>