On Wed, Jun 06, 2018 at 04:58:46PM -0400, Jeff King wrote:
> 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.
I used it because it's already in use earlier in the file in some of the
mingw_* functions. Perhaps we happen to know that our mingw systems
will always have a suitable /bin/sh, but I suppose some less capable
shells would still have choked on it by now.
I can remove it if necessary, but it didn't seem necessary.
> > > + 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).
This was exactly my reasoning.
> > > + if [ "$1" = "-f" ]
> >
> > Style nit, please avoid [] and use test:
> > if test "$1" = "-f"
>
> This I agree with. :)
Yeah, I forgot that that's our style. I'll fix that.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature
