From: Brad King <brad.king@kitware.com>
Date: Mon, 02 Sep 2013 17:23:42 GMT
Subject: Re: [PATCH v2 8/8] update-ref: add test cases covering --stdin signature
Message-ID: <5224C99E.3050209@kitware.com>
In-Reply-To: <CAPig+cQhH+t6unkea=0O2neJm8gOFaiKzPdSnuBaRy=Sdruu=g@mail.gmail.com>

On 08/31/2013 11:41 PM, Eric Sunshine wrote:
>> +       rm -f stdin &&
>> +       touch stdin &&
> 
> Unless the timestamp of 'stdin' has particular significance, modern
> git tests avoid 'touch' in favor of creating the empty file like this
> 
>     >stdin &&

Fixed.

>> +       git update-ref --stdin < stdin &&
> 
> Style: Git test scripts omit whitespace following <, >, <<, and >>.

Fixed.

>> +test_expect_success 'stdin fails with bad line lines' '
> 
> Despite the semantic relationship between all these cases, if there is
> a regression in one case, the person reading the verbose output has to
> study it carefully to determine the offending case. If you decompose
> this monolith so that each case is in its own test_expect_success,
> then the regressed case becomes immediately obvious.

Yes, of course.  Fixed.

> multi-line preparations of 'stdin' might be more readable with a heredoc:
> 
>     cat >stdin <<-EOF &&
>     $a $m
>     $b $m
>     $a $m
>     EOF

Fixed.

Thanks,
-Brad