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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to