From: Jonathan Nieder Date: Fri, 09 Dec 2011 20:07:03 GMT Subject: Re: [PATCH 1/2] t3401: modernize style Message-ID: <20111209200703.GA21280@elie.hsd1.il.comcast.net> In-Reply-To: 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: >> +       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. [...] >> +       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