Kaartic Sivaraam <kaarticsivaraam91...@gmail.com> writes:
> The sample hook to prepare the commit message before
> a commit allows users to opt-in to add the signature
> to the commit message. The signature is added at a place
> that isn't consistent with the "-s" option of "git commit".
> Further, it could go out of view in certain cases.
>
> Add the signature in a way similar to "-s" option of
> "git commit" using git's interpret-trailers command.
OK.
> It works well in all cases except when the user invokes
> "git commit" without any arguments. In that case manually
> add a new line after the first line to ensure it's consistent
> with the output of "-s" option.
OK (but its execution can be simplified).
> While at it, name the input parameters to improve readability
> of script.
Good.
>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91...@gmail.com>
> ---
> Added a close quote that got missed in the previous patch.
>
> templates/hooks--prepare-commit-msg.sample | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/templates/hooks--prepare-commit-msg.sample
> b/templates/hooks--prepare-commit-msg.sample
> index 86b8f227e..4ddab7896 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -20,17 +20,28 @@
> # The third example adds a Signed-off-by line to the message, that can
> # still be edited. This is rarely a good idea.
>
> -case "$2,$3" in
> +COMMIT_MSG_FILE=$1
> +COMMIT_SOURCE=$2
> +SHA1=$3
> +NEW_LINE='\
> +'
> +SED_TEMP_FILE='.sed-output.temp'
Move the latter two variable definitions to where they matter,
i.e. the commented-out part that computes and adds $SOB.
> +case "$COMMIT_SOURCE,$SHA1" in
> merge,)
> - @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/;
> print' "$1" ;;
> + @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/;
> print' "$COMMIT_MSG_FILE" ;;
>
> # ,|template,)
> # @PERL_PATH@ -i.bak -pe '
> # print "\n" . `git diff --cached --name-status -r`
> -# if /^#/ && $first++ == 0' "$1" ;;
> +# if /^#/ && $first++ == 0' "$COMMIT_MSG_FILE" ;;
>
> *) ;;
> esac
>
> # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by:
> \1/p')
> -# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
> +# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
> +# if [ -z "$COMMIT_SOURCE" ]
> +# then
In our codebase (see Documentation/CodingGuidelines) we tend to
prefer "test" over "[". I.e.
if test -z "$COMMIT_SOURCE"
then
> +# sed -e "1i$NEW_LINE" "$COMMIT_MSG_FILE" >"$SED_TEMP_FILE" && mv
> "$SED_TEMP_FILE" "$COMMIT_MSG_FILE"
This looks like a more complex way to say
{
echo
cat "$COMMIT_MSG_FILE"
} >"$SED_TEMP_FILE" &&
mv "$SED_TEMP_FILE" "$COMMIT_MSG_FILE"
I wondered if it is safe to use a single hardcoded "$SED_TEMP_FILE",
but I think it is OK.
> +# fi
Thanks.
