Web lists-archives.com

Re: [PATCH 01/10] t: add tool to translate hash-related values




On Wed, Jun 06, 2018 at 08:19:27AM +0200, Torsten Bögershausen wrote:

> > +test_translate_f_ () {
> > +	local file="$TEST_DIRECTORY/translate/$2" &&
> 
> Unless I'm wrong, we don't use the "local" keyword ?

We've got a test balloon out; see 01d3a526ad (t0000: check whether the
shell supports the "local" keyword, 2017-10-26). I think it's reasonable
to consider starting its use.

> > +	perl -e '
> 
> The bare "perl" is better spelled as "$PERL_PATH"

This use is OK. Since a0e0ec9f7d (t: provide a perl() function which
uses $PERL_PATH, 2013-10-28), most common uses handle this automatically
(there are some exceptions, covered in t/README).

> > +	if [ "$1" = "-f" ]
> 
> Style nit, please avoid [] and use test:
> 	if test "$1" = "-f"

This I agree with. :)

-Peff