Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes: > Modernize the style of all tests throughout the file: > - Remove spurious blank lines. > - Indent the test body. > - Make sure that all lines end with &&, to make it easier to spot breaks > in the chain. > - When executing something in a subshell, put the parenthesis on > separate lines and indent the body. Also make sure that the first > statement in the subshell is a 'cd'. > - When redirecting output, replace the > output forms with >output. > - Use the <<-\EOF and <<-EOF forms of heredoc, not <<EOF. Also, don't > de-indent the heredoc body. > - When creating an empty file, use : >output form over >output for > clarity. Everything except the last one is good, I think. For the last one, I prefer ">output" moderately over ": >output", as both are equally clear to people who write shell scripts, but this preference is not strong enough to make me change ": >output" that is originally in the file to ">output" in a patch. In the very old days, there were some implementations of shells that mishandled ">output" but those days are long gone. > Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx> > --- > t/t5505-remote.sh | 813 +++++++++++++++++++++++++++++------------------------- > 1 file changed, 440 insertions(+), 373 deletions(-) > > -cat > test/expect << EOF > ... > +cat >test/expect <<-EOF > * remote origin > Fetch URL: $(pwd)/one > Push URL: $(pwd)/one This one is questionable; if it is not indented with HT, there is no point using the -EOF form. > -cat > test/expect << EOF > ... > +cat >test/expect <<-EOF Likewise. > @@ -219,152 +220,187 @@ cat > test/expect << EOF > EOF > > test_expect_success 'show -n' ' > - (mv one one.unreachable && > - cd test && > ... > + mv one one.unreachable && > + ( > + cd test && This is more than an indentation change, but I think it is a good one. I saw some others that moved "mkdir" out of the subshell to chdir into that directory, which are also good changes. > cat >test/expect <<\EOF Funny that you did not touch this one. > -cat > one/expect << EOF > +cat >one/expect <<-\EOF > apis/master But this again is an unnecessary use of <<- This smells like a largely blind conversion done with a script, which needs a human proof-reader other than the submitter; everything else looked good. I'll queue with this fix-up squashed in. t/t5505-remote.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index f679ded..0e7dfa2 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -107,7 +107,7 @@ test_expect_success C_LOCALE_OUTPUT 'remove remote' ' check_remote_track origin master side && git for-each-ref "--format=%(refname)" refs/remotes | sed -e "/^refs\/remotes\/origin\//d" >actual && - : >expect && + >expect && test_cmp expect actual ) ' @@ -139,7 +139,7 @@ test_expect_success 'remove remote protects local branches' ' ) ' -cat >test/expect <<-EOF +cat >test/expect <<EOF * remote origin Fetch URL: $(pwd)/one Push URL: $(pwd)/one @@ -202,7 +202,7 @@ test_expect_success 'show' ' ) ' -cat >test/expect <<-EOF +cat >test/expect <<EOF * remote origin Fetch URL: $(pwd)/one Push URL: $(pwd)/one @@ -262,7 +262,7 @@ test_expect_success 'set-head --auto' ' ) ' -cat >test/expect <<-\EOF +cat >test/expect <<\EOF error: Multiple remote HEAD branches. Please choose one explicitly with: git remote set-head two another git remote set-head two master @@ -276,7 +276,7 @@ test_expect_success 'set-head --auto fails w/multiple HEADs' ' ) ' -cat >test/expect <<-\EOF +cat >test/expect <<\EOF refs/remotes/origin/side2 EOF @@ -290,7 +290,7 @@ test_expect_success 'set-head explicit' ' ) ' -cat >test/expect <<-EOF +cat >test/expect <<EOF Pruning origin URL: $(pwd)/one * [would prune] origin/side2 @@ -483,7 +483,7 @@ EOF test_expect_success 'add with reachable tags (default)' ' ( cd one && - : >foobar && + >foobar && git add foobar && git commit -m "Foobar" && git tag -a -m "Foobar tag" foobar-tag && @@ -551,7 +551,7 @@ test_expect_success 'reject --no-no-tags' ' ) ' -cat >one/expect <<-\EOF +cat >one/expect <<\EOF apis/master apis/side drosophila/another @@ -570,7 +570,7 @@ test_expect_success 'update' ' ) ' -cat >one/expect <<-\EOF +cat >one/expect <<\EOF drosophila/another drosophila/master drosophila/side @@ -637,7 +637,7 @@ test_expect_success 'update default' ' ) ' -cat >one/expect <<-\EOF +cat >one/expect <<\EOF drosophila/another drosophila/master drosophila/side @@ -732,7 +732,7 @@ test_expect_success 'rename a remote with name prefix of other remote' ' ) ' -cat >remotes_origin <<-EOF +cat >remotes_origin <<EOF URL: $(pwd)/one Push: refs/heads/master:refs/heads/upstream Pull: refs/heads/master:refs/heads/origin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html