From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 09 Jan 2022 18:32:21 GMT
Subject: Re: [PATCH] hook: test -a|o is not POSIX
Message-ID: <xmqqmtk43glm.fsf@gitster.g>
In-Reply-To: <9c7aeb5b-87b2-a51e-c08d-8dfc75ecf588@gmail.com>

Bagas Sanjaya <bagasdotme@gmail.com> writes:

>> -		if [ "$oldrev" = "$zero" -a "$denycreatebranch" = "true" ]; then
>> +		if [ "$oldrev" = "$zero" ] && [ "$denycreatebranch" = "true" ]; then
>>   			echo "*** Creating a branch is not allowed in this repository" >&2
>>   			exit 1
>>   		fi
>> base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907
>
> From the patch above, the title should be "replace non-POSIX test -a & test -o with logical operator".

I think "should" is a bit too strong a word.  While yours is a bit
more explicit on _what_ the solution is, "hook: test -a|o is not
POSIX" already implies that we are fixing that non-posix-ness by
rewriting, and it is obvious (cf. Documentation/CodingGuidelines)
what the right rewrite should be.

One thing the original does a bit better tha yours is that it tell
us _where_ the problem is.  So, perhaps

    sample hook: use "test ... &&/|| test ..." instead of "test -a/-o"

But I find the original just fine.