Re: [PATCH v2 01/14] t/t5505-remote: modernize style
Ramkumar Ramachandra artag...@gmail.com 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 artag...@gmail.com --- 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 --
Re: [PATCH v2 01/14] t/t5505-remote: modernize style
Junio C Hamano wrote: This smells like a largely blind conversion done with a script, It is :) I'll queue with this fix-up squashed in. Thanks; I really appreciate it. -- 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
[PATCH v2 01/14] t/t5505-remote: modernize style
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.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
t/t5505-remote.sh | 813 +-
1 file changed, 440 insertions(+), 373 deletions(-)
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index dd10ff0..f679ded 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -42,107 +42,104 @@ check_tracking_branch () {
}
test_expect_success setup '
-
setup_repository one
setup_repository two
(
- cd two git branch another
+ cd two
+ git branch another
)
git clone one test
-
'
test_expect_success C_LOCALE_OUTPUT 'remote information for the origin' '
-(
- cd test
- tokens_match origin $(git remote)
- check_remote_track origin master side
- check_tracking_branch origin HEAD master side
-)
+ (
+ cd test
+ tokens_match origin $(git remote)
+ check_remote_track origin master side
+ check_tracking_branch origin HEAD master side
+ )
'
test_expect_success 'add another remote' '
-(
- cd test
- git remote add -f second ../two
- tokens_match origin second $(git remote)
- check_tracking_branch second master side another
- git for-each-ref --format=%(refname) refs/remotes |
- sed -e /^refs\/remotes\/origin\//d \
- -e /^refs\/remotes\/second\//d actual
- expect
- test_cmp expect actual
-)
+ (
+ cd test
+ git remote add -f second ../two
+ tokens_match origin second $(git remote)
+ check_tracking_branch second master side another
+ git for-each-ref --format=%(refname) refs/remotes |
+ sed -e /^refs\/remotes\/origin\//d \
+ -e /^refs\/remotes\/second\//d actual
+ expect
+ test_cmp expect actual
+ )
'
test_expect_success C_LOCALE_OUTPUT 'check remote tracking' '
-(
- cd test
- check_remote_track origin master side
- check_remote_track second master side another
-)
+ (
+ cd test
+ check_remote_track origin master side
+ check_remote_track second master side another
+ )
'
test_expect_success 'remote forces tracking branches' '
-(
- cd test
- case `git config remote.second.fetch` in
- +*) true ;;
-*) false ;;
- esac
-)
+ (
+ cd test
+ case `git config remote.second.fetch` in
+ +*) true ;;
+*) false ;;
+ esac
+ )
'
test_expect_success 'remove remote' '
-(
- cd test
- git symbolic-ref refs/remotes/second/HEAD refs/remotes/second/master
- git remote rm second
-)
+ (
+ cd test
+ git symbolic-ref refs/remotes/second/HEAD
refs/remotes/second/master
+ git remote rm second
+ )
'
test_expect_success C_LOCALE_OUTPUT 'remove remote' '
-(
- cd test
- tokens_match origin $(git remote)
- check_remote_track origin master side
- git for-each-ref --format=%(refname) refs/remotes |
- sed -e /^refs\/remotes\/origin\//d actual
- expect
- test_cmp expect actual
-)
+ (
+ cd test
+ tokens_match origin $(git remote)
+ check_remote_track origin master side
+ git for-each-ref --format=%(refname) refs/remotes |
+ sed -e /^refs\/remotes\/origin\//d actual
+ : expect
+ test_cmp expect actual
+ )
'
test_expect_success 'remove remote protects local branches' '
-(
- cd test
- { cat expect1 EOF
-Note: A branch outside the refs/remotes/ hierarchy was not removed;
-to delete it, use:
- git branch -d master
-EOF
- }
- { cat expect2 EOF
-Note: Some branches outside the refs/remotes/ hierarchy were not removed;
-to delete them, use:
- git branch -d foobranch
- git branch -d master
-EOF
- }
- git tag footag
- git config --add remote.oops.fetch +refs/*:refs/*
- git remote remove oops 2actual1
- git branch foobranch
- git config --add remote.oops.fetch +refs/*:refs/*
- git remote rm oops 2actual2
-
