Re: [PATCH v3 1/3] tests: Prepare --textconv tests for correctly-failing conversion program

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 28, 2010 at 11:58:45AM -0400, Jeff King wrote:
> On Tue, Sep 28, 2010 at 08:36:46AM -0700, Junio C Hamano wrote:
> 
> > >> >  cat >helper <<'EOF'
> > >> >  #!/bin/sh
> > >> > -sed 's/^/converted: /' "$@" >helper.out
> > >> > +grep -q '^bin: ' "$@" || { echo "E: $@ is not \"binary\" file" 1>&2; exit 1; }
> > >>  ...
> > > ...
> > > I too think (1) is right. It was just that originally there was $@
> > > (which I now understand was wrong).
> > 
> > Well, the original's use of "$@" is perfectly fine; it would do the right
> > thing with one argument, of course, but it would do the right thing with
> > more than one, too.  On the other hand, your use inside "echo" is not.
> 
> Moreover, the use of "grep" is wrong. Giving it two files, one of which
> has "^bin: " and one of which doesn't, will silently accept the latter.
> If it's going to handle multiple files, it must be a for-loop (or you
> could invert "grep -qv", but I think that might be getting too clever to
> remain readable).
> 
> Which is why I suggested just dropping the t4042 bit, which is the only
> part that actually needs to handle multiple arguments. The other ones
> can just switch to using "$1". The helper script is simple enough that
> there is no need for them to share the same code.

Let's do whatever is appropriate. I just wanted to be consitent, but I'm
doing mistakes becase I'm overworked and have time pressure on other
tasks. (sorry).

If it would be more convenient to just drop t4042 - let's do it.

I could amend the patch, or is it ok to just kill that portion on top of
what is already in pu?


Sorry for dummy me,
Kirill
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]