Re: [PATCH v2 0/1] templates: pre-push hook: check for missing GPG signatures

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 25, 2013, 16:50 UTC
Message-ID: <7vy5c64krz.fsf@alter.siamese.dyndns.org>
In-Reply-To: <51791F41.3040203@physik.tu-berlin.de>

Sebastian Götte <jaseg@physik.tu-berlin.de> writes:

> On 04/24/2013 09:54 PM, Junio C Hamano wrote:
>> None of the above is part of a proper commit log message, is it?
> Fixed (I hope)

Don't hope, instead do.  ;-)

The questions I asked were not requests to explain them to _me_ in a response like this. They were the examples of what the proposed commit log message should have explained what the patch attempts to do.

Show 5 quoted lines

>>> -IFS=' '
>> Why?
> Otherwise in the for-loop below the output of the pipe chain is not correctly
> split by newlines. Also AFAIK, this is not needed: I think the default
> '<space><tab><newline>' is just fine here.

It is not enough to make sure that IFS has SP so that existing code works correctly; we also need to see if the existing code needs to avoid cutting the tokens at HT or LF. I think in this case using the default IFS is safe, as input to pre-push are SP separated refs and object names, none of which can have SP, HT or LF in it.

Show 13 quoted lines

>># Check for missing good GPG signatures
>>git log --format="%G? %h" "$range" |
>>(
>>                exitcode=0
>>                while read sign commit
>>                do
>>                                test "$sign" = G && continue
>>                                echo "Commit $commit does not have a good GPG signature"
>>                                exitcode=1
>>                done
>>                exit $exitcode
>>)
>>let exitcode=exitcode\|$?

Don't use bash-ism "let".

The above loop is a perfectly fine and readable way to write the logic, by the way

Except that we tend to prefer $ret over $exitcode, but I've already said that.

Thanks.