On Mon, Jul 16, 2018 at 5:30 PM Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> > +test_expect_success 'reduce_heads' '
> > + cat >input <<-\EOF &&
> > + X:commit-1-10
> > + X:commit-2-8
> > + X:commit-3-6
> > + X:commit-4-4
> > + X:commit-1-7
> > + X:commit-2-5
> > + X:commit-3-3
> > + X:commit-5-1
> > + EOF
> > + {
> > + printf "reduce_heads(X):\n" &&
> > + git rev-parse commit-5-1 &&
> > + git rev-parse commit-4-4 &&
> > + git rev-parse commit-3-6 &&
> > + git rev-parse commit-2-8 &&
> > + git rev-parse commit-1-10
> > + } >expect &&
>
> Please use rev-parse only once.
>
> I am not sure about the usage of { braces } in the test suite,
> +cc Eric who sent a test suite linting series recently.
> Do we need to em-'brace' the statements that describe the
> expected behavior? (Or is it supposed to be easier to read
> for the reviewers? I found these very readable so far... but
> this question just came up)
Grouping the commands for redirection via a "{...}>expect" block is
less noisy than redirecting each command separately, thus more
reviewer-friendly. And, {...} blocks are used regularly in the test
suite, so no issue there.
I do agree that a single git-rev-parse with all 5 arguments makes more
sense (and would be appreciated by Windows folk). Also, the 'printf'
could be replaced by a simple 'echo' if we want to get nit-picky.
Finally, a style nit: We don't normally indent the content of a
here-doc like that. Instead, the content is normally aligned with the
closing EOF, not indented beyond it.
