Re: [PATCH v2] bugreport: collect list of populated hooks

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 28, 2020, 00:04 UTC
Message-ID: <xmqqh7x45vd1.fsf@gitster.c.googlers.com>
In-Reply-To: <20200427234510.GC36078@google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 16 quoted lines

> Emily Shaffer wrote:
>
>> --- a/t/t0091-bugreport.sh
>> +++ b/t/t0091-bugreport.sh
>> @@ -57,5 +57,15 @@ test_expect_success 'can create leading directories outside of a git dir' '
>>  	nongit git bugreport -o foo/bar/baz
>>  '
>>  
>> +test_expect_success 'indicates populated hooks' '
>> +	test_when_finished rm git-bugreport-hooks.txt &&
>> +	test_when_finished rm -fr .git/hooks &&
>> +	mkdir .git/hooks &&
>> +	touch .git/hooks/applypatch-msg &&
>> +	chmod +x .git/hooks/applypatch-msg &&
>
> optional: could use write_script for this

Yup, it is a good practice to do so. Use of touch is especially bad here, as it is quite plausible for us to later change the "is the hook there?" check to exclude a completely empty file.

Show 5 quoted lines

>> +	git bugreport -s hooks &&
>> +	test_i18ngrep applypatch-msg git-bugreport-hooks.txt
>
> This should work even when translated, so it can use "grep" instead of
> test_i18ngrep.

Nicely spotted.

Thanks.