[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH v1] t6300: fix issues related to %(contents:size)
From:       Jeff King <peff () peff ! net>
Date:       2020-07-31 20:30:04
Message-ID: 20200731203004.GA1440843 () coredump ! intra ! peff ! net
[Download RAW message or body]

On Fri, Jul 31, 2020 at 01:04:10PM -0700, Junio C Hamano wrote:

> > Definitely fixes the issue, though I wonder:
> >
> >> -		echo $expect >expected
> >> -		test_expect_${4:-sucess} $PREREQ "basic atom: $1 contents:size" '
> >> +		echo $expect >expect
> >> +		test_expect_${4:-success} $PREREQ "basic atom: $1 contents:size" '
> >>  			git for-each-ref --format="%(contents:size)" "$ref" >actual &&
> >>  			test_cmp expect actual
> >>  		'
> >
> > Should we instead switch the test_cmp to look at "expected" to be
> > consistent with the rest of the tests in this file?
> 
> If I recall correctly, "expect vs actual" were more common when I
> counted across all the tests last time.  Matching local convention
> is fine, though.

Yes, I agree that "expect" is where we should be heading overall. I
think matching local convention is best here to avoid introducing new
mistakes like this one, but I wouldn't be opposed to somebody switching
out s/expected/expect/ in the whole file.

-Peff
[prev in list] [next in list] [prev in thread] [next in thread]