On Mon, May 9, 2016 at 12:22 PM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Mon, May 9, 2016 at 12:09 PM, Armin Kunaschik
> <megabr...@googlemail.com> wrote:
>> *** t4151-am-abort.sh Mon May 9 17:51:44 2016
>> --- t4151-am-abort.sh.orig Fri Apr 29 23:37:00 2016
>> ! test 3 -eq "$(git ls-files -u | wc -l)" &&
>> ! test 3 -eq $(git ls-files -u | wc -l) &&
>
> Some comments:
>
> Quoting the output of 'wc -l' will break the tests on Mac OS X and BSD
> since the output contains leading whitespace which won't match the "3"
> on the other side of the '='.
This isn't quite accurate. Since the test is using '-eq' rather than
'=', the leading whitespace won't be a problem, but it can still be
problematic if you're somehow getting an empty result from the
pipeline:
% test 3 -eq "$(echo)"
-bash: test: : integer expression expected
> Your diff is backward, comparing 'current' against 'original', which
> makes it difficult to read. Reviewers on this list expect to see
> 'original' compared against 'current'.
>
> Use a unified format to make the diff easier to read; or just use
> git-diff or git-format patch, which is even simpler.
>
> It's not clear how the output of 'wc -l' could ever be the empty
> string. Perhaps git-ls-files is dying and causing the pipe to abort
> before 'wc -l' ever outputs anything? Without additional information
> about the problem you're experiencing, it's difficult to judge if this
> change is a good idea.
--
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
