Subject: Re: [PATCH] hook: test -a|o is not POSIX

From: Junio C Hamano <[email-masked]>
Date: Sun, 09 Jan 2022 10:32:21 -0800

Bagas Sanjaya <[email-masked]> 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.