{"thread":{"id":"44689","subject":"[PATCH] t3600: slightly modernize style","startedAt":"2016-12-12T23:55:06Z","lastAt":"2016-12-13T07:11:55Z","messageCount":2,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"307560","messageId":"20161212235455.13796-1-sbeller@google.com","threadId":"44689","inReplyTo":null,"subject":"[PATCH] t3600: slightly modernize style","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-12T23:54:55Z","receivedAt":"2016-12-12T23:55:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Remove the space between redirection and file name.\nAlso remove unnecessary invocations of subshells, such as\n\n\t(cd submod &&\n\t\techo X >untracked\n\t) &&\n\nas there is no point of having the shell for functional purposes.\nIn case of a single Git command use the `-C` option to let Git cd into\nthe directory.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/t3600-rm.sh | 122 ++++++++++++++++++++++++----------------------------------\n 1 file changed, 50 insertions(+), 72 deletions(-)\n \n I came across this test script while developing the checkout series.\n This motivated me to modernize the style while reading it.\n \n The removal of spawning subshells for just one command could be argued to be\n a performance gain as well, though I did not measure it.\n \n Thanks,\n Stefan \n\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 14f0edca2b..5e5a16c863 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -111,21 +111,21 @@ test_expect_success 'Remove nonexistent file with --ignore-unmatch' '\n '\n \n test_expect_success '\"rm\" command printed' '\n-\techo frotz > test-file &&\n+\techo frotz >test-file &&\n \tgit add test-file &&\n \tgit commit -m \"add file for rm test\" &&\n-\tgit rm test-file > rm-output &&\n+\tgit rm test-file >rm-output &&\n \ttest $(grep \"^rm \" rm-output | wc -l) = 1 &&\n \trm -f test-file rm-output &&\n \tgit commit -m \"remove file from rm test\"\n '\n \n test_expect_success '\"rm\" command suppressed with --quiet' '\n-\techo frotz > test-file &&\n+\techo frotz >test-file &&\n \tgit add test-file &&\n \tgit commit -m \"add file for rm --quiet test\" &&\n-\tgit rm --quiet test-file > rm-output &&\n-\ttest $(wc -l < rm-output) = 0 &&\n+\tgit rm --quiet test-file >rm-output &&\n+\ttest_must_be_empty rm-output &&\n \trm -f test-file rm-output &&\n \tgit commit -m \"remove file from rm --quiet test\"\n '\n@@ -221,7 +221,7 @@ test_expect_success 'Call \"rm\" from outside the work tree' '\n \tmkdir repo &&\n \t(cd repo &&\n \t git init &&\n-\t echo something > somefile &&\n+\t echo something >somefile &&\n \t git add somefile &&\n \t git commit -m \"add a file\" &&\n \t (cd .. &&\n@@ -287,7 +287,7 @@ test_expect_success 'rm removes empty submodules from work tree' '\n \tgit commit -m \"add submodule\" &&\n \tgit rm submod &&\n \ttest ! -e submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.url &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.path\n@@ -298,7 +298,7 @@ test_expect_success 'rm removes removed submodule from index and .gitmodules' '\n \tgit submodule update &&\n \trm -rf submod &&\n \tgit rm submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.url &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.path\n@@ -309,7 +309,7 @@ test_expect_success 'rm removes work tree of unmodified submodules' '\n \tgit submodule update &&\n \tgit rm submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.url &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.path\n@@ -320,7 +320,7 @@ test_expect_success 'rm removes a submodule with a trailing /' '\n \tgit submodule update &&\n \tgit rm submod/ &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -335,17 +335,15 @@ test_expect_success 'rm succeeds when given a directory with a trailing /' '\n test_expect_success 'rm of a populated submodule with different HEAD fails unless forced' '\n \tgit reset --hard &&\n \tgit submodule update &&\n-\t(cd submod &&\n-\t\tgit checkout HEAD^\n-\t) &&\n+\tgit -C submod checkout HEAD^ &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.modified actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.url &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.path\n@@ -418,34 +416,30 @@ test_expect_success 'rm issues a warning when section is not found in .gitmodule\n test_expect_success 'rm of a populated submodule with modifications fails unless forced' '\n \tgit reset --hard &&\n \tgit submodule update &&\n-\t(cd submod &&\n-\t\techo X >empty\n-\t) &&\n+\techo X >submod/empty &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.modified actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'rm of a populated submodule with untracked files fails unless forced' '\n \tgit reset --hard &&\n \tgit submodule update &&\n-\t(cd submod &&\n-\t\techo X >untracked\n-\t) &&\n+\techo X >submod/untracked &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.modified actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -461,16 +455,12 @@ test_expect_success 'setup submodule conflict' '\n \tgit add nitfol &&\n \tgit commit -m \"added nitfol 2\" &&\n \tgit checkout -b conflict1 master &&\n-\t(cd submod &&\n-\t\tgit fetch &&\n-\t\tgit checkout branch1\n-\t) &&\n+\tgit -C submod fetch &&\n+\tgit -C submod checkout branch1 &&\n \tgit add submod &&\n \tgit commit -m \"submod 1\" &&\n \tgit checkout -b conflict2 master &&\n-\t(cd submod &&\n-\t\tgit checkout branch2\n-\t) &&\n+\tgit -C submod checkout branch2 &&\n \tgit add submod &&\n \tgit commit -m \"submod 2\"\n '\n@@ -486,7 +476,7 @@ test_expect_success 'rm removes work tree of unmodified conflicted submodule' '\n \ttest_must_fail git merge conflict2 &&\n \tgit rm submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -494,18 +484,16 @@ test_expect_success 'rm of a conflicted populated submodule with different HEAD\n \tgit checkout conflict1 &&\n \tgit reset --hard &&\n \tgit submodule update &&\n-\t(cd submod &&\n-\t\tgit checkout HEAD^\n-\t) &&\n+\tgit -C submod checkout HEAD^ &&\n \ttest_must_fail git merge conflict2 &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.conflict actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.url &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.path\n@@ -515,18 +503,16 @@ test_expect_success 'rm of a conflicted populated submodule with modifications f\n \tgit checkout conflict1 &&\n \tgit reset --hard &&\n \tgit submodule update &&\n-\t(cd submod &&\n-\t\techo X >empty\n-\t) &&\n+\techo X >submod/empty &&\n \ttest_must_fail git merge conflict2 &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.conflict actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.url &&\n \ttest_must_fail git config -f .gitmodules submodule.sub.path\n@@ -536,18 +522,16 @@ test_expect_success 'rm of a conflicted populated submodule with untracked files\n \tgit checkout conflict1 &&\n \tgit reset --hard &&\n \tgit submodule update &&\n-\t(cd submod &&\n-\t\techo X >untracked\n-\t) &&\n+\techo X >submod/untracked &&\n \ttest_must_fail git merge conflict2 &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.conflict actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -564,12 +548,12 @@ test_expect_success 'rm of a conflicted populated submodule with a .git director\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -d submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.conflict actual &&\n \ttest_must_fail git rm -f submod &&\n \ttest -d submod &&\n \ttest -d submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.conflict actual &&\n \tgit merge --abort &&\n \trm -rf submod\n@@ -581,7 +565,7 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '\n \ttest_must_fail git merge conflict2 &&\n \tgit rm submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -597,12 +581,12 @@ test_expect_success 'rm of a populated submodule with a .git directory fails eve\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -d submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \t! test -s actual &&\n \ttest_must_fail git rm -f submod &&\n \ttest -d submod &&\n \ttest -d submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \t! test -s actual &&\n \trm -rf submod\n '\n@@ -629,58 +613,52 @@ test_expect_success 'setup subsubmodule' '\n test_expect_success 'rm recursively removes work tree of unmodified submodules' '\n \tgit rm submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'rm of a populated nested submodule with different nested HEAD fails unless forced' '\n \tgit reset --hard &&\n \tgit submodule update --recursive &&\n-\t(cd submod/subsubmod &&\n-\t\tgit checkout HEAD^\n-\t) &&\n+\tgit -C submod/subsubmod checkout HEAD^ &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.modified actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'rm of a populated nested submodule with nested modifications fails unless forced' '\n \tgit reset --hard &&\n \tgit submodule update --recursive &&\n-\t(cd submod/subsubmod &&\n-\t\techo X >empty\n-\t) &&\n+\techo X >submod/subsubmod/empty &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.modified actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'rm of a populated nested submodule with nested untracked files fails unless forced' '\n \tgit reset --hard &&\n \tgit submodule update --recursive &&\n-\t(cd submod/subsubmod &&\n-\t\techo X >untracked\n-\t) &&\n+\techo X >submod/subsubmod/untracked &&\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -f submod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect.modified actual &&\n \tgit rm -f submod &&\n \ttest ! -d submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -695,12 +673,12 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc\n \ttest_must_fail git rm submod &&\n \ttest -d submod &&\n \ttest -d submod/subsubmod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \t! test -s actual &&\n \ttest_must_fail git rm -f submod &&\n \ttest -d submod &&\n \ttest -d submod/subsubmod/.git &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \t! test -s actual &&\n \trm -rf submod\n '\n@@ -716,7 +694,7 @@ test_expect_success 'checking out a commit after submodule removal needs manual\n \techo \"?? submod/\" >expected &&\n \ttest_cmp expected actual &&\n \trm -rf submod &&\n-\tgit status -s -uno --ignore-submodules=none > actual &&\n+\tgit status -s -uno --ignore-submodules=none >actual &&\n \t! test -s actual\n '\n \n-- \n2.11.0.rc2.55.g1345a36.dirty\n\n"},{"id":"307576","messageId":"xmqqvauo5mmk.fsf@gitster.mtv.corp.google.com","threadId":"44689","inReplyTo":"20161212235455.13796-1-sbeller@google.com","subject":"Re: [PATCH] t3600: slightly modernize style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-13T07:11:47Z","receivedAt":"2016-12-13T07:11:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Remove the space between redirection and file name.\n> Also remove unnecessary invocations of subshells, such as\n>\n> \t(cd submod &&\n> \t\techo X >untracked\n> \t) &&\n>\n> as there is no point of having the shell for functional purposes.\n> In case of a single Git command use the `-C` option to let Git cd into\n> the directory.\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n\nLooks good.  Thanks.\n"}]}