Re: [PATCH 1/2] t3401: modernize style
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Dec 9, 2011, 20:07 UTC
- Message-ID
- <20111209200703.GA21280@elie.hsd1.il.comcast.net>
- In-Reply-To
- <CALkWK0mip_pzUDQO=YLxaVwsWEAUdrk_EKcNG94Xr5+N+kzBtw@mail.gmail.com>
Ramkumar Ramachandra wrote:
> The motivation is unclear: lazy afternoon? :P
Perhaps he was reading the list and after noticing a few patches in the same vein, realized that this test script could be made easier to read, too.
[...]
> Martin von Zweigbergk wrote:
Show 5 quoted lines
>> + echo First > A && >> + git update-index --add A && >> + git commit -m "Add A." && > > Style nit: >[^ ] is prevalent FWIW.
At first it wasn't clear to me what you meant here. Was it that quoted text in an email should start with a non-space character, like a tab?
Finally I caught on that you mean that redirection operators tend to be flush against the filename they are redirecting to.
[...]
Show 5 quoted lines
>> + test ! -d .git/rebase-apply >> +' > > While at it, why not change this "test ! -d" to > "test_path_is_missing"?
Sounds like a useful hint. The benefits are that it would catch failures that make .git/rebase-apply into an ordinary file, and more useful output from "sh t3401-* -v -i" when the test fails. The main downside I can think of is that the test script would not run against versions of the test harness before v1.7.3.3~5^2~1 (test-lib: user-friendly alternatives to test [-d|-f|-e], 2010-08-10).
The patch looks good to me, too. Thanks, both.
Sincerely, Jonathan