Re: [PATCHv2] add: ignore only ignored files

From: Jeff King <peff@peff.net>
Date: Nov 23, 2014, 19:50 UTC
Message-ID: <20141123195041.GA16854@peff.net>
In-Reply-To: <5470FE0A.1030802@web.de>

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.

Show 5 quoted lines

> 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