Hi, Taylor Blau wrote: > A number of spots in t5537 use the non-indented heredoc '<<EOF' when > they would benefit from instead using '<<-EOF' or simply > test_write_lines. > > In preparation for adding new tests in a good style and being consistent > with the surrounding code, update the existing tests to improve their > readability. > > Suggested-by: Junio C Hamano <gitster@xxxxxxxxx> > Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx> > --- > t/t5537-fetch-shallow.sh | 70 +++++++++++----------------------------- > 1 file changed, 18 insertions(+), 52 deletions(-) Sounds like a good idea. Some nitpicks --- please don't act on them all, but only the ones that seem appropriate to you: [...] > +++ b/t/t5537-fetch-shallow.sh > @@ -25,10 +25,7 @@ test_expect_success 'setup' ' > test_expect_success 'setup shallow clone' ' > git clone --no-local --depth=2 .git shallow && > git --git-dir=shallow/.git log --format=%s >actual && > - cat <<EOF >expect && > -4 > -3 > -EOF > + test_write_lines 4 3 >expect && > test_cmp expect actual > ' Nice. [...] > @@ -133,14 +110,12 @@ test_expect_success 'fetch that requires changes in .git/shallow is filtered' ' [...] > - cat <<EOF >expect && > -no-shallow > -EOF > + cat <<-EOF >expect && > + no-shallow > + EOF Can this use "echo"? Or if using cat, please quote the EOF in <<-EOF so the reader doesn't have to check for $substitutions in the body: cat >expect <<-\EOF && [...] > @@ -158,21 +133,15 @@ test_expect_success 'fetch --update-shallow' ' > git fetch --update-shallow ../shallow/.git refs/heads/*:refs/remotes/shallow/* && > git fsck && > git for-each-ref --sort=refname --format="%(refname)" >actual.refs && > - cat <<EOF >expect.refs && > -refs/remotes/shallow/master > -refs/remotes/shallow/no-shallow > -refs/tags/heavy-tag > -refs/tags/light-tag > -EOF > + cat <<-EOF >expect.refs && Likewise (missing \ before EOF). A few more nits, that probably don't belong in the same patch: - the code in subshells would be more readable if indented - existing <<-EOF here blocks should \quote the EOF - the resulting history would be more realistic if it uses test_tick before running "git commit". Or perhaps this can use the test_commit helper to handle that - should use test_must_fail in preference to ! git - might be simpler if http tests go in a different file Thanks, Jonathan
