Subject: Re: [PATCHv2] add: ignore only ignored files

From: Jeff King <[email-masked]>
Date: Sun, 23 Nov 2014 14:50:42 -0500

On Sat, Nov 22, 2014 at 10:20:10PM +0100, Torsten Bögershausen wrote:

> > I do not think there is a real _downside_ to using test_must_fail for
> > grep, except that it is a bit more verbose.
> We may burn CPU cycles 

It adds a single if/else chain. If your shell does not implement that as
a fast builtin, you have bigger performance problems. :)

> I counted 19 "test_must_fail grep" under t/*sh, and 201 "! grep".

I do not mind a patch to fix them, but with the usual caveat of avoiding
stepping on the toes of any topics in flight. It is also fine to leave
them until the area is touched.

> As a general rule for further review of shell scripts can we say ?
> ! git                # incorrect, we don't capture e.g. segfaults of signal 
> test_must_fail grep  # correct, but not preferred for new code
> ! grep               # preferred for new code
> test_must_fail git   # correct

I think that's true. The snippet from t/README Junio quoted lays it out
pretty clearly, I think. If you didn't know there was documentation in
t/README that was worth reading before writing tests, then that is the
thing I think should go in CodingGuidelines.

-Peff