git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5][Outreachy] modernizing the test scripts
@ 2020-10-15 17:57 charvi-077
  2020-10-15 17:57 ` [PATCH 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting charvi-077
                   ` (6 more replies)
  0 siblings, 7 replies; 60+ messages in thread
From: charvi-077 @ 2020-10-15 17:57 UTC (permalink / raw)
  To: git; +Cc: christian.couder, phillip.wood123, sunshine, charvi-077

This is my first patch series to the GIT mailing list. I followed the
link[1] and t7001 patches to modernize and clean up the test scripts.

This patch series : 
 -modernize the three test scripts : t7101 , t7201 and t102. 
 - cleans up with 5 types of changes in all the three scripts.
   1. Converting the old old style test format to new one
   2. Removing blankspaces in test bodies 
   3. Removing whitespaces after the redirect operator, according to
      Codingguidelines .  
   4. Using git -C instead of cd 
   5. Placing all commands in seperate lines. 

Also, I have tested the scripts and set up travis CI[2].
[1]https://lore.kernel.org/git/CAPig+cQpUu2UO-+jWn1nTaDykWnxwuEitzVB7PnW2SS_b7V8Hg@mail.gmail.com/
[2]https://travis-ci.org/github/charvi-077/git/branches


charvi-077 (5):
  t7101,t7102,t7201: modernize test formatting
  t7102,t7201: remove unnecessary blank spaces in test body
  t7102,t7201: remove whitespace after redirect operator
  t7201: avoid using cd outside of subshells
  t7201: place each command in its own line

 t/t7101-reset-empty-subdirs.sh |  66 ++++++++++-----------
 t/t7102-reset.sh               |  63 ++++++++------------
 t/t7201-co.sh                  | 102 +++++++++++++--------------------
 3 files changed, 96 insertions(+), 135 deletions(-)

-- 
2.29.0.rc1


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting
  2020-10-15 17:57 [PATCH 0/5][Outreachy] modernizing the test scripts charvi-077
@ 2020-10-15 17:57 ` charvi-077
  2020-10-16 13:07   ` Christian Couder
  2020-10-15 17:57 ` [PATCH 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body charvi-077
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 60+ messages in thread
From: charvi-077 @ 2020-10-15 17:57 UTC (permalink / raw)
  To: git; +Cc: christian.couder, phillip.wood123, sunshine, charvi-077

Some tests in this script are formatted using a very old style:
        test_expect_success \
            'title' \
            'body line 1 &&
            body line 2'

Updating the formatting to the modern style:
        test_expect_success 'title' '
            body line 1 &&
            body line 2
        '
Signed-off-by: charvi-077 <charvi077@gmail.com>
---
 t/t7101-reset-empty-subdirs.sh | 66 +++++++++++++++++-----------------
 t/t7102-reset.sh               | 24 +++++--------
 t/t7201-co.sh                  | 31 ++++++++--------
 3 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/t/t7101-reset-empty-subdirs.sh b/t/t7101-reset-empty-subdirs.sh
index 96e163f084..bfce05ac5d 100755
--- a/t/t7101-reset-empty-subdirs.sh
+++ b/t/t7101-reset-empty-subdirs.sh
@@ -6,16 +6,15 @@
 test_description='git reset should cull empty subdirs'
 . ./test-lib.sh
 
-test_expect_success \
-    'creating initial files' \
-    'mkdir path0 &&
+test_expect_success 'creating initial files' '
+     mkdir path0 &&
      cp "$TEST_DIRECTORY"/../COPYING path0/COPYING &&
      git add path0/COPYING &&
-     git commit -m add -a'
+     git commit -m add -a
+'
 
-test_expect_success \
-    'creating second files' \
-    'mkdir path1 &&
+test_expect_success 'creating second files' '
+     mkdir path1 &&
      mkdir path1/path2 &&
      cp "$TEST_DIRECTORY"/../COPYING path1/path2/COPYING &&
      cp "$TEST_DIRECTORY"/../COPYING path1/COPYING &&
@@ -25,39 +24,40 @@ test_expect_success \
      git add path1/COPYING &&
      git add COPYING &&
      git add path0/COPYING-TOO &&
-     git commit -m change -a'
+     git commit -m change -a
+'
 
-test_expect_success \
-    'resetting tree HEAD^' \
-    'git reset --hard HEAD^'
+test_expect_success 'resetting tree HEAD^' '
+     git reset --hard HEAD^
+'
 
-test_expect_success \
-    'checking initial files exist after rewind' \
-    'test -d path0 &&
-     test -f path0/COPYING'
+test_expect_success 'checking initial files exist after rewind' '
+     test -d path0 &&
+     test -f path0/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/path2/COPYING' \
-    '! test -f path1/path2/COPYING'
+test_expect_success 'checking lack of path1/path2/COPYING' '
+    ! test -f path1/path2/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/COPYING' \
-    '! test -f path1/COPYING'
+test_expect_success 'checking lack of path1/COPYING' '
+    ! test -f path1/COPYING
+'
 
-test_expect_success \
-    'checking lack of COPYING' \
-    '! test -f COPYING'
+test_expect_success 'checking lack of COPYING' '
+     ! test -f COPYING
+'
 
-test_expect_success \
-    'checking checking lack of path1/COPYING-TOO' \
-    '! test -f path0/COPYING-TOO'
+test_expect_success 'checking checking lack of path1/COPYING-TOO' '
+     ! test -f path0/COPYING-TOO
+'
 
-test_expect_success \
-    'checking lack of path1/path2' \
-    '! test -d path1/path2'
+test_expect_success 'checking lack of path1/path2' '
+     ! test -d path1/path2
+'
 
-test_expect_success \
-    'checking lack of path1' \
-    '! test -d path1'
+test_expect_success 'checking lack of path1' '
+     ! test -d path1
+'
 
 test_done
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 22161b3b2d..fe43f77513 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -107,8 +107,7 @@ test_expect_success 'reset --soft with unmerged index should fail' '
 	git rm --cached -- un
 '
 
-test_expect_success \
-	'giving paths with options different than --mixed should fail' '
+test_expect_success 'giving paths with options different than --mixed should fail' '
 	test_must_fail git reset --soft -- first &&
 	test_must_fail git reset --hard -- first &&
 	test_must_fail git reset --soft HEAD^ -- first &&
@@ -128,8 +127,7 @@ test_expect_success 'giving unrecognized options should fail' '
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending merge should fail' '
+test_expect_success 'trying to do reset --soft with pending merge should fail' '
 	git branch branch1 &&
 	git branch branch2 &&
 
@@ -152,8 +150,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending checkout merge should fail' '
+test_expect_success 'trying to do reset --soft with pending checkout merge should fail' '
 	git branch branch3 &&
 	git branch branch4 &&
 
@@ -175,8 +172,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'resetting to HEAD with no changes should succeed and do nothing' '
+test_expect_success 'resetting to HEAD with no changes should succeed and do nothing' '
 	git reset --hard &&
 		check_changes $head5 &&
 	git reset --hard HEAD &&
@@ -226,8 +222,7 @@ secondfile:
 2nd line 2nd file
 3rd line 2nd file
 EOF
-test_expect_success \
-	'changing files and redo the last commit should succeed' '
+test_expect_success 'changing files and redo the last commit should succeed' '
 	echo "3rd line 2nd file" >>secondfile &&
 	git commit -a -C ORIG_HEAD &&
 	head4=$(git rev-parse --verify HEAD) &&
@@ -245,8 +240,7 @@ first:
 second:
 2nd file
 EOF
-test_expect_success \
-	'--hard reset should change the files and undo commits permanently' '
+test_expect_success '--hard reset should change the files and undo commits permanently' '
 	git reset --hard HEAD~2 &&
 	check_changes $head5p2 &&
 	test "$(git rev-parse ORIG_HEAD)" = \
@@ -284,8 +278,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'redoing changes adding them without commit them should succeed' '
+test_expect_success 'redoing changes adding them without commit them should succeed' '
 	git rm first &&
 	git mv second secondfile &&
 
@@ -380,8 +373,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'--hard reset to ORIG_HEAD should clear a fast-forward merge' '
+test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge' '
 	git reset --hard HEAD^ &&
 	check_changes $head5 &&
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 4d62b9b00f..a800bda5e3 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -62,7 +62,7 @@ test_expect_success setup '
 	git checkout master
 '
 
-test_expect_success "checkout from non-existing branch" '
+test_expect_success 'checkout from non-existing branch' '
 
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
@@ -71,7 +71,7 @@ test_expect_success "checkout from non-existing branch" '
 	test refs/heads/master = "$(git symbolic-ref HEAD)"
 '
 
-test_expect_success "checkout with dirty tree without -m" '
+test_expect_success 'checkout with dirty tree without -m' '
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
@@ -84,7 +84,7 @@ test_expect_success "checkout with dirty tree without -m" '
 
 '
 
-test_expect_success "checkout with unrelated dirty tree without -m" '
+test_expect_success 'checkout with unrelated dirty tree without -m' '
 
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
@@ -95,7 +95,7 @@ test_expect_success "checkout with unrelated dirty tree without -m" '
 	test_cmp messages.expect messages
 '
 
-test_expect_success "checkout -m with dirty tree" '
+test_expect_success 'checkout -m with dirty tree' '
 
 	git checkout -f master &&
 	git clean -f &&
@@ -120,7 +120,7 @@ test_expect_success "checkout -m with dirty tree" '
 	test_must_be_empty current.index
 '
 
-test_expect_success "checkout -m with dirty tree, renamed" '
+test_expect_success 'checkout -m with dirty tree, renamed' '
 
 	git checkout -f master && git clean -f &&
 
@@ -388,22 +388,22 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 '
 
-test_expect_success \
-    'checkout w/--track sets up tracking' '
+test_expect_success 'checkout w/--track sets up tracking' '
     git config branch.autosetupmerge false &&
     git checkout master &&
     git checkout --track -b track1 &&
     test "$(git config branch.track1.remote)" &&
-    test "$(git config branch.track1.merge)"'
+    test "$(git config branch.track1.merge)"
+'
 
-test_expect_success \
-    'checkout w/autosetupmerge=always sets up tracking' '
+test_expect_success 'checkout w/autosetupmerge=always sets up tracking' '
     test_when_finished git config branch.autosetupmerge false &&
     git config branch.autosetupmerge always &&
     git checkout master &&
     git checkout -b track2 &&
     test "$(git config branch.track2.remote)" &&
-    test "$(git config branch.track2.merge)"'
+    test "$(git config branch.track2.merge)"
+'
 
 test_expect_success 'checkout w/--track from non-branch HEAD fails' '
     git checkout master^0 &&
@@ -435,8 +435,7 @@ test_expect_success 'detach a symbolic link HEAD' '
     test "z$(git rev-parse --verify refs/heads/master)" = "z$here"
 '
 
-test_expect_success \
-    'checkout with --track fakes a sensible -b <name>' '
+test_expect_success 'checkout with --track fakes a sensible -b <name>' '
     git config remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" &&
     git update-ref refs/remotes/origin/koala/bear renamer &&
 
@@ -457,9 +456,9 @@ test_expect_success \
     test "$(git rev-parse HEAD)" = "$(git rev-parse renamer)"
 '
 
-test_expect_success \
-    'checkout with --track, but without -b, fails with too short tracked name' '
-    test_must_fail git checkout --track renamer'
+test_expect_success 'checkout with --track, but without -b, fails with too short tracked name' '
+    test_must_fail git checkout --track renamer
+'
 
 setup_conflicting_index () {
 	rm -f .git/index &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body
  2020-10-15 17:57 [PATCH 0/5][Outreachy] modernizing the test scripts charvi-077
  2020-10-15 17:57 ` [PATCH 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting charvi-077
@ 2020-10-15 17:57 ` charvi-077
  2020-10-15 17:57 ` [PATCH 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator charvi-077
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 60+ messages in thread
From: charvi-077 @ 2020-10-15 17:57 UTC (permalink / raw)
  To: git; +Cc: christian.couder, phillip.wood123, sunshine, charvi-077

Some tests use a deprecated style in which there are unnecessary blank lines after the opening quote of the test body and before the closing quote. So we should removed these unnecessary blank lines.

Signed-off-by: charvi-077 <charvi077@gmail.com>
---
 t/t7102-reset.sh |  9 ---------
 t/t7201-co.sh    | 25 -------------------------
 2 files changed, 34 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index fe43f77513..2b4cfb2c83 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -439,7 +439,6 @@ test_expect_success 'test --mixed <paths>' '
 '
 
 test_expect_success 'test resetting the index at give paths' '
-
 	mkdir sub &&
 	>sub/file1 &&
 	>sub/file2 &&
@@ -452,7 +451,6 @@ test_expect_success 'test resetting the index at give paths' '
 	echo "$U" &&
 	test_must_fail git diff-index --cached --exit-code "$T" &&
 	test "$T" != "$U"
-
 '
 
 test_expect_success 'resetting an unmodified path is a no-op' '
@@ -490,7 +488,6 @@ test_expect_success 'resetting specific path that is unmerged' '
 '
 
 test_expect_success 'disambiguation (1)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -499,11 +496,9 @@ test_expect_success 'disambiguation (1)' '
 	test -z "$(git diff --cached --name-only)" &&
 	test -f secondfile &&
 	test_must_be_empty secondfile
-
 '
 
 test_expect_success 'disambiguation (2)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -511,11 +506,9 @@ test_expect_success 'disambiguation (2)' '
 	test_must_fail git reset secondfile &&
 	test -n "$(git diff --cached --name-only -- secondfile)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (3)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -524,11 +517,9 @@ test_expect_success 'disambiguation (3)' '
 	test_must_fail git diff --quiet &&
 	test -z "$(git diff --cached --name-only)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (4)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index a800bda5e3..b527f8009c 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,6 @@ fill () {
 
 
 test_expect_success setup '
-
 	fill x y z > same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
@@ -63,7 +62,6 @@ test_expect_success setup '
 '
 
 test_expect_success 'checkout from non-existing branch' '
-
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
 	test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
@@ -72,7 +70,6 @@ test_expect_success 'checkout from non-existing branch' '
 '
 
 test_expect_success 'checkout with dirty tree without -m' '
-
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
 	then
@@ -81,11 +78,9 @@ test_expect_success 'checkout with dirty tree without -m' '
 	else
 		echo "happy - failed correctly"
 	fi
-
 '
 
 test_expect_success 'checkout with unrelated dirty tree without -m' '
-
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
 	cp same kept &&
@@ -96,7 +91,6 @@ test_expect_success 'checkout with unrelated dirty tree without -m' '
 '
 
 test_expect_success 'checkout -m with dirty tree' '
-
 	git checkout -f master &&
 	git clean -f &&
 
@@ -121,7 +115,6 @@ test_expect_success 'checkout -m with dirty tree' '
 '
 
 test_expect_success 'checkout -m with dirty tree, renamed' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 2 3 4 5 7 8 >one &&
@@ -139,11 +132,9 @@ test_expect_success 'checkout -m with dirty tree, renamed' '
 	! test -f one &&
 	git diff --cached >current &&
 	test_must_be_empty current
-
 '
 
 test_expect_success 'checkout -m with merge conflict' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 T 3 4 5 6 S 8 >one &&
@@ -166,7 +157,6 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-
 	git checkout -f master && git clean -f &&
 
 	fill b d > two &&
@@ -190,7 +180,6 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 
 	fill b d > two &&
@@ -216,7 +205,6 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 	git rm two &&
 
@@ -228,7 +216,6 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 '
 
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
-
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
 	git checkout -f renamer && git clean -f &&
@@ -267,7 +254,6 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-
 	git checkout -f master && git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -283,7 +269,6 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-
 	git checkout -f master && git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -299,7 +284,6 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-
 	git checkout -f master && git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -315,7 +299,6 @@ test_expect_success 'checkout to detach HEAD with HEAD^0' '
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git tag both side &&
 	git branch both master &&
 	git reset --hard &&
@@ -327,11 +310,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	test "z$H" = "z$M" &&
 	name=$(git symbolic-ref HEAD 2>/dev/null) &&
 	test "z$name" = zrefs/heads/both
-
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -351,11 +332,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	else
 		: happy
 	fi
-
 '
 
 test_expect_success 'switch branches while in subdirectory' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -366,11 +345,9 @@ test_expect_success 'switch branches while in subdirectory' '
 	) &&
 	! test -f subs/one &&
 	rm -fr subs
-
 '
 
 test_expect_success 'checkout specific path while in subdirectory' '
-
 	git reset --hard &&
 	git checkout side &&
 	mkdir subs &&
@@ -385,7 +362,6 @@ test_expect_success 'checkout specific path while in subdirectory' '
 		git checkout side -- bero
 	) &&
 	test -f subs/bero
-
 '
 
 test_expect_success 'checkout w/--track sets up tracking' '
@@ -608,7 +584,6 @@ test_expect_success 'failing checkout -b should not break working tree' '
 	test $(git symbolic-ref HEAD) = refs/heads/master &&
 	git diff --exit-code &&
 	git diff --cached --exit-code
-
 '
 
 test_expect_success 'switch out of non-branch' '
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-15 17:57 [PATCH 0/5][Outreachy] modernizing the test scripts charvi-077
  2020-10-15 17:57 ` [PATCH 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting charvi-077
  2020-10-15 17:57 ` [PATCH 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body charvi-077
@ 2020-10-15 17:57 ` charvi-077
  2020-10-15 17:57 ` [PATCH 4/5][Outreachy] t7201: avoid using cd outside of subshells charvi-077
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 60+ messages in thread
From: charvi-077 @ 2020-10-15 17:57 UTC (permalink / raw)
  To: git; +Cc: christian.couder, phillip.wood123, sunshine, charvi-077

According to Documentation/CodingGuidelines, there should be no whitespace after redirect operators. So, we should remove these whitespaces after redirect operators.

Signed-off-by: charvi-077 <charvi077@gmail.com>
---
 t/t7102-reset.sh | 30 +++++++++++++++---------------
 t/t7201-co.sh    | 10 +++++-----
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 2b4cfb2c83..a8c96bf162 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -70,15 +70,15 @@ check_changes () {
 
 test_expect_success 'reset --hard message' '
 	hex=$(git log -1 --format="%h") &&
-	git reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg) > .expected &&
+	git reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
 test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' '
 	hex=$(git log -1 --format="%h") &&
-	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg $test_encoding) > .expected &&
+	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg $test_encoding) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
@@ -387,25 +387,25 @@ test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge
 '
 
 test_expect_success 'test --mixed <paths>' '
-	echo 1 > file1 &&
-	echo 2 > file2 &&
+	echo 1 >file1 &&
+	echo 2 >file2 &&
 	git add file1 file2 &&
 	test_tick &&
 	git commit -m files &&
 	before1=$(git rev-parse --short HEAD:file1) &&
 	before2=$(git rev-parse --short HEAD:file2) &&
 	git rm file2 &&
-	echo 3 > file3 &&
-	echo 4 > file4 &&
-	echo 5 > file1 &&
+	echo 3 >file3 &&
+	echo 4 >file4 &&
+	echo 5 >file1 &&
 	after1=$(git rev-parse --short $(git hash-object file1)) &&
 	after4=$(git rev-parse --short $(git hash-object file4)) &&
 	git add file1 file3 file4 &&
 	git reset HEAD -- file1 file2 file3 &&
 	test_must_fail git diff --quiet &&
-	git diff > output &&
+	git diff >output &&
 
-	cat > expect <<-EOF &&
+	cat >expect <<-EOF &&
 	diff --git a/file1 b/file1
 	index $before1..$after1 100644
 	--- a/file1
@@ -423,9 +423,9 @@ test_expect_success 'test --mixed <paths>' '
 	EOF
 
 	test_cmp expect output &&
-	git diff --cached > output &&
+	git diff --cached >output &&
 
-	cat > cached_expect <<-EOF &&
+	cat >cached_expect <<-EOF &&
 	diff --git a/file4 b/file4
 	new file mode 100644
 	index 0000000..$after4
@@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
 	git diff-index --cached --exit-code HEAD
 '
 
-cat > expect << EOF
+cat >expect << EOF
 Unstaged changes after reset:
 M	file2
 EOF
 
 test_expect_success '--mixed refreshes the index' '
 	echo 123 >> file2 &&
-	git reset --mixed HEAD > output &&
+	git reset --mixed HEAD >output &&
 	test_i18ncmp expect output
 '
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index b527f8009c..74553f991b 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,7 @@ fill () {
 
 
 test_expect_success setup '
-	fill x y z > same &&
+	fill x y z >same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
 	git add same one two &&
@@ -55,7 +55,7 @@ test_expect_success setup '
 
 	git checkout -b simple master &&
 	rm -f one &&
-	fill a c e > two &&
+	fill a c e >two &&
 	git commit -a -m "Simple D one, M two" &&
 
 	git checkout master
@@ -95,7 +95,7 @@ test_expect_success 'checkout -m with dirty tree' '
 	git clean -f &&
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
-	git checkout -m side > messages &&
+	git checkout -m side >messages &&
 
 	test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
 
@@ -159,7 +159,7 @@ test_expect_success 'checkout -m with merge conflict' '
 test_expect_success 'format of merge conflict from checkout -m' '
 	git checkout -f master && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout -m simple &&
 
 	git ls-files >current &&
@@ -182,7 +182,7 @@ test_expect_success 'format of merge conflict from checkout -m' '
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 	git checkout -f master && git reset --hard && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
 
 	cat <<-EOF >expect &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-15 17:57 [PATCH 0/5][Outreachy] modernizing the test scripts charvi-077
                   ` (2 preceding siblings ...)
  2020-10-15 17:57 ` [PATCH 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator charvi-077
@ 2020-10-15 17:57 ` charvi-077
  2020-10-15 17:57 ` [PATCH 5/5][Outreachy] t7201: place each command in its own line charvi-077
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 60+ messages in thread
From: charvi-077 @ 2020-10-15 17:57 UTC (permalink / raw)
  To: git; +Cc: christian.couder, phillip.wood123, sunshine, charvi-077

Avoid using `cd` outside of subshells since, if the test fails, there is no guarantee that the current working directory is the expected one, which may cause subsequent tests to run in the wrong directory.

Signed-off-by: charvi-077 <charvi077@gmail.com>
---
 t/t7201-co.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 74553f991b..5898182fd2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
 	git checkout master &&
 
 	mkdir subs &&
-	(
-		cd subs &&
-		git checkout side
-	) &&
+	git -C subs checkout side &&
 	! test -f subs/one &&
 	rm -fr subs
 '
@@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 	git checkout master &&
 	mkdir -p subs &&
-	(
-		cd subs &&
-		git checkout side -- bero
-	) &&
+	git -C subs checkout side -- bero &&
 	test -f subs/bero
 '
 
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 5/5][Outreachy] t7201: place each command in its own line
  2020-10-15 17:57 [PATCH 0/5][Outreachy] modernizing the test scripts charvi-077
                   ` (3 preceding siblings ...)
  2020-10-15 17:57 ` [PATCH 4/5][Outreachy] t7201: avoid using cd outside of subshells charvi-077
@ 2020-10-15 17:57 ` charvi-077
  2020-10-16 12:54 ` [PATCH 0/5][Outreachy] modernizing the test scripts Christian Couder
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
  6 siblings, 0 replies; 60+ messages in thread
From: charvi-077 @ 2020-10-15 17:57 UTC (permalink / raw)
  To: git; +Cc: christian.couder, phillip.wood123, sunshine, charvi-077

Multiple commands on one line should be split across multiple lines.

Signed-off-by: charvi-077 <charvi077@gmail.com>
---
 t/t7201-co.sh | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5898182fd2..b36a93056f 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -157,7 +157,8 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout -m simple &&
@@ -180,7 +181,9 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
@@ -205,7 +208,9 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 	git rm two &&
 
 	test_must_fail git checkout simple 2>errs &&
@@ -218,7 +223,8 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	git checkout renamer^ 2>messages &&
 	test_i18ngrep "HEAD is now at $rev" messages &&
 	test_line_count = 1 messages &&
@@ -237,7 +243,8 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '
 test_expect_success 'checkout to detach HEAD' '
 	git config advice.detachedHead true &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
 	grep "HEAD is now at $rev" messages &&
 	test_line_count -gt 1 messages &&
@@ -254,7 +261,8 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -269,7 +277,8 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -284,7 +293,8 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH 0/5][Outreachy] modernizing the test scripts
  2020-10-15 17:57 [PATCH 0/5][Outreachy] modernizing the test scripts charvi-077
                   ` (4 preceding siblings ...)
  2020-10-15 17:57 ` [PATCH 5/5][Outreachy] t7201: place each command in its own line charvi-077
@ 2020-10-16 12:54 ` Christian Couder
  2020-10-17  8:27   ` Charvi Mendiratta
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
  6 siblings, 1 reply; 60+ messages in thread
From: Christian Couder @ 2020-10-16 12:54 UTC (permalink / raw)
  To: charvi-077; +Cc: git, phillip.wood123, Eric Sunshine

On Thu, Oct 15, 2020 at 7:57 PM charvi-077 <charvi077@gmail.com> wrote:
>
> This is my first patch series to the GIT mailing list. I followed the
> link[1] and t7001 patches to modernize and clean up the test scripts.

Thanks for getting started contributing!

> This patch series :
>  -modernize the three test scripts : t7101, t7201 and t102.

s/t102/t7102/

On https://git.github.io/Outreachy-21-Microprojects/ we say:

"Find one test script that needs some of the same changes and make them."

So working only one test script, for example only t7101, would have
been better than working on 3 test scripts. Now that you started
working on 3 test scripts, it's ok to finish modernizing all these 3
test scripts though.

>  - cleans up with 5 types of changes in all the three scripts.
>    1. Converting the old old style test format to new one

s/old old/old/

>    2. Removing blankspaces in test bodies
>    3. Removing whitespaces after the redirect operator, according to
>       Codingguidelines .

s/Codingguidelines/CodingGuidelines/

>    4. Using git -C instead of cd
>    5. Placing all commands in seperate lines.

s/seperate/separate/

> Also, I have tested the scripts and set up travis CI[2].

Nice!

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting
  2020-10-15 17:57 ` [PATCH 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting charvi-077
@ 2020-10-16 13:07   ` Christian Couder
  0 siblings, 0 replies; 60+ messages in thread
From: Christian Couder @ 2020-10-16 13:07 UTC (permalink / raw)
  To: charvi-077; +Cc: git, phillip.wood123, Eric Sunshine

On Thu, Oct 15, 2020 at 7:58 PM charvi-077 <charvi077@gmail.com> wrote:
>
> Some tests in this script are formatted using a very old style:

s/this script/these scripts/

as this patch is modifying more than one test script.

>         test_expect_success \
>             'title' \
>             'body line 1 &&
>             body line 2'
>
> Updating the formatting to the modern style:
>         test_expect_success 'title' '
>             body line 1 &&
>             body line 2
>         '
> Signed-off-by: charvi-077 <charvi077@gmail.com>

As Junio already mentioned we request that contributors use their full
real name as the author name (which is by default the sender of the
email) and in the "Signed-off-by: "

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v2 0/5][Outreachy] modernizing the test scripts
  2020-10-15 17:57 [PATCH 0/5][Outreachy] modernizing the test scripts charvi-077
                   ` (5 preceding siblings ...)
  2020-10-16 12:54 ` [PATCH 0/5][Outreachy] modernizing the test scripts Christian Couder
@ 2020-10-17  7:54 ` Charvi Mendiratta
  2020-10-17  7:54   ` [PATCH v2 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
                     ` (6 more replies)
  6 siblings, 7 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-17  7:54 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, sunshine, Charvi Mendiratta

Thanks to Junio and Christian for corrections required in previous patch series, I have updated them in this patch series. I have updated the author name and sign-offs on all the patches, and also corrected the mistakes in cover letter and commit message of patch. 

This patch series :
 -modernize the three test scripts : t7101, t7201 and t7102.
 - cleans up with 5 types of changes in all the three scripts.
   1. Converting the old style test format to new one
   2. Removing blankspaces in test bodies
   3. Removing whitespaces after the redirect operator, according to
      CodingGuidelines 
   4. Using git -C instead of cd
   5. Placing all commands in separate lines


Charvi Mendiratta (5):
  t7101,t7102,t7201: modernize test formatting
  t7102,t7201: remove unnecessary blank spaces in test body
  t7102,t7201: remove whitespace after redirect operator
  t7201: avoid using cd outside of subshells
  t7201: place each command in its own line

 t/t7101-reset-empty-subdirs.sh |  66 ++++++++++-----------
 t/t7102-reset.sh               |  63 ++++++++------------
 t/t7201-co.sh                  | 102 +++++++++++++--------------------
 3 files changed, 96 insertions(+), 135 deletions(-)

-- 
2.29.0.rc1


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v2 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
@ 2020-10-17  7:54   ` Charvi Mendiratta
  2020-10-17  7:54   ` [PATCH v2 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-17  7:54 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, sunshine, Charvi Mendiratta

Some tests in these scripts are formatted using a very old style:
        test_expect_success \
            'title' \
            'body line 1 &&
            body line 2'

Updating the formatting to the modern style:
        test_expect_success 'title' '
            body line 1 &&
            body line 2
        '

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7101-reset-empty-subdirs.sh | 66 +++++++++++++++++-----------------
 t/t7102-reset.sh               | 24 +++++--------
 t/t7201-co.sh                  | 31 ++++++++--------
 3 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/t/t7101-reset-empty-subdirs.sh b/t/t7101-reset-empty-subdirs.sh
index 96e163f084..bfce05ac5d 100755
--- a/t/t7101-reset-empty-subdirs.sh
+++ b/t/t7101-reset-empty-subdirs.sh
@@ -6,16 +6,15 @@
 test_description='git reset should cull empty subdirs'
 . ./test-lib.sh
 
-test_expect_success \
-    'creating initial files' \
-    'mkdir path0 &&
+test_expect_success 'creating initial files' '
+     mkdir path0 &&
      cp "$TEST_DIRECTORY"/../COPYING path0/COPYING &&
      git add path0/COPYING &&
-     git commit -m add -a'
+     git commit -m add -a
+'
 
-test_expect_success \
-    'creating second files' \
-    'mkdir path1 &&
+test_expect_success 'creating second files' '
+     mkdir path1 &&
      mkdir path1/path2 &&
      cp "$TEST_DIRECTORY"/../COPYING path1/path2/COPYING &&
      cp "$TEST_DIRECTORY"/../COPYING path1/COPYING &&
@@ -25,39 +24,40 @@ test_expect_success \
      git add path1/COPYING &&
      git add COPYING &&
      git add path0/COPYING-TOO &&
-     git commit -m change -a'
+     git commit -m change -a
+'
 
-test_expect_success \
-    'resetting tree HEAD^' \
-    'git reset --hard HEAD^'
+test_expect_success 'resetting tree HEAD^' '
+     git reset --hard HEAD^
+'
 
-test_expect_success \
-    'checking initial files exist after rewind' \
-    'test -d path0 &&
-     test -f path0/COPYING'
+test_expect_success 'checking initial files exist after rewind' '
+     test -d path0 &&
+     test -f path0/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/path2/COPYING' \
-    '! test -f path1/path2/COPYING'
+test_expect_success 'checking lack of path1/path2/COPYING' '
+    ! test -f path1/path2/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/COPYING' \
-    '! test -f path1/COPYING'
+test_expect_success 'checking lack of path1/COPYING' '
+    ! test -f path1/COPYING
+'
 
-test_expect_success \
-    'checking lack of COPYING' \
-    '! test -f COPYING'
+test_expect_success 'checking lack of COPYING' '
+     ! test -f COPYING
+'
 
-test_expect_success \
-    'checking checking lack of path1/COPYING-TOO' \
-    '! test -f path0/COPYING-TOO'
+test_expect_success 'checking checking lack of path1/COPYING-TOO' '
+     ! test -f path0/COPYING-TOO
+'
 
-test_expect_success \
-    'checking lack of path1/path2' \
-    '! test -d path1/path2'
+test_expect_success 'checking lack of path1/path2' '
+     ! test -d path1/path2
+'
 
-test_expect_success \
-    'checking lack of path1' \
-    '! test -d path1'
+test_expect_success 'checking lack of path1' '
+     ! test -d path1
+'
 
 test_done
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 22161b3b2d..fe43f77513 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -107,8 +107,7 @@ test_expect_success 'reset --soft with unmerged index should fail' '
 	git rm --cached -- un
 '
 
-test_expect_success \
-	'giving paths with options different than --mixed should fail' '
+test_expect_success 'giving paths with options different than --mixed should fail' '
 	test_must_fail git reset --soft -- first &&
 	test_must_fail git reset --hard -- first &&
 	test_must_fail git reset --soft HEAD^ -- first &&
@@ -128,8 +127,7 @@ test_expect_success 'giving unrecognized options should fail' '
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending merge should fail' '
+test_expect_success 'trying to do reset --soft with pending merge should fail' '
 	git branch branch1 &&
 	git branch branch2 &&
 
@@ -152,8 +150,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending checkout merge should fail' '
+test_expect_success 'trying to do reset --soft with pending checkout merge should fail' '
 	git branch branch3 &&
 	git branch branch4 &&
 
@@ -175,8 +172,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'resetting to HEAD with no changes should succeed and do nothing' '
+test_expect_success 'resetting to HEAD with no changes should succeed and do nothing' '
 	git reset --hard &&
 		check_changes $head5 &&
 	git reset --hard HEAD &&
@@ -226,8 +222,7 @@ secondfile:
 2nd line 2nd file
 3rd line 2nd file
 EOF
-test_expect_success \
-	'changing files and redo the last commit should succeed' '
+test_expect_success 'changing files and redo the last commit should succeed' '
 	echo "3rd line 2nd file" >>secondfile &&
 	git commit -a -C ORIG_HEAD &&
 	head4=$(git rev-parse --verify HEAD) &&
@@ -245,8 +240,7 @@ first:
 second:
 2nd file
 EOF
-test_expect_success \
-	'--hard reset should change the files and undo commits permanently' '
+test_expect_success '--hard reset should change the files and undo commits permanently' '
 	git reset --hard HEAD~2 &&
 	check_changes $head5p2 &&
 	test "$(git rev-parse ORIG_HEAD)" = \
@@ -284,8 +278,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'redoing changes adding them without commit them should succeed' '
+test_expect_success 'redoing changes adding them without commit them should succeed' '
 	git rm first &&
 	git mv second secondfile &&
 
@@ -380,8 +373,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'--hard reset to ORIG_HEAD should clear a fast-forward merge' '
+test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge' '
 	git reset --hard HEAD^ &&
 	check_changes $head5 &&
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 4d62b9b00f..a800bda5e3 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -62,7 +62,7 @@ test_expect_success setup '
 	git checkout master
 '
 
-test_expect_success "checkout from non-existing branch" '
+test_expect_success 'checkout from non-existing branch' '
 
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
@@ -71,7 +71,7 @@ test_expect_success "checkout from non-existing branch" '
 	test refs/heads/master = "$(git symbolic-ref HEAD)"
 '
 
-test_expect_success "checkout with dirty tree without -m" '
+test_expect_success 'checkout with dirty tree without -m' '
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
@@ -84,7 +84,7 @@ test_expect_success "checkout with dirty tree without -m" '
 
 '
 
-test_expect_success "checkout with unrelated dirty tree without -m" '
+test_expect_success 'checkout with unrelated dirty tree without -m' '
 
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
@@ -95,7 +95,7 @@ test_expect_success "checkout with unrelated dirty tree without -m" '
 	test_cmp messages.expect messages
 '
 
-test_expect_success "checkout -m with dirty tree" '
+test_expect_success 'checkout -m with dirty tree' '
 
 	git checkout -f master &&
 	git clean -f &&
@@ -120,7 +120,7 @@ test_expect_success "checkout -m with dirty tree" '
 	test_must_be_empty current.index
 '
 
-test_expect_success "checkout -m with dirty tree, renamed" '
+test_expect_success 'checkout -m with dirty tree, renamed' '
 
 	git checkout -f master && git clean -f &&
 
@@ -388,22 +388,22 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 '
 
-test_expect_success \
-    'checkout w/--track sets up tracking' '
+test_expect_success 'checkout w/--track sets up tracking' '
     git config branch.autosetupmerge false &&
     git checkout master &&
     git checkout --track -b track1 &&
     test "$(git config branch.track1.remote)" &&
-    test "$(git config branch.track1.merge)"'
+    test "$(git config branch.track1.merge)"
+'
 
-test_expect_success \
-    'checkout w/autosetupmerge=always sets up tracking' '
+test_expect_success 'checkout w/autosetupmerge=always sets up tracking' '
     test_when_finished git config branch.autosetupmerge false &&
     git config branch.autosetupmerge always &&
     git checkout master &&
     git checkout -b track2 &&
     test "$(git config branch.track2.remote)" &&
-    test "$(git config branch.track2.merge)"'
+    test "$(git config branch.track2.merge)"
+'
 
 test_expect_success 'checkout w/--track from non-branch HEAD fails' '
     git checkout master^0 &&
@@ -435,8 +435,7 @@ test_expect_success 'detach a symbolic link HEAD' '
     test "z$(git rev-parse --verify refs/heads/master)" = "z$here"
 '
 
-test_expect_success \
-    'checkout with --track fakes a sensible -b <name>' '
+test_expect_success 'checkout with --track fakes a sensible -b <name>' '
     git config remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" &&
     git update-ref refs/remotes/origin/koala/bear renamer &&
 
@@ -457,9 +456,9 @@ test_expect_success \
     test "$(git rev-parse HEAD)" = "$(git rev-parse renamer)"
 '
 
-test_expect_success \
-    'checkout with --track, but without -b, fails with too short tracked name' '
-    test_must_fail git checkout --track renamer'
+test_expect_success 'checkout with --track, but without -b, fails with too short tracked name' '
+    test_must_fail git checkout --track renamer
+'
 
 setup_conflicting_index () {
 	rm -f .git/index &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v2 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
  2020-10-17  7:54   ` [PATCH v2 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
@ 2020-10-17  7:54   ` Charvi Mendiratta
  2020-10-17 15:13     ` Đoàn Trần Công Danh
  2020-10-17  7:54   ` [PATCH v2 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-17  7:54 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, sunshine, Charvi Mendiratta

Some tests use a deprecated style in which there are unnecessary blank lines after the opening quote of the test body and before the closing quote. So we should remove these unnecessary blank lines.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7102-reset.sh |  9 ---------
 t/t7201-co.sh    | 25 -------------------------
 2 files changed, 34 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index fe43f77513..2b4cfb2c83 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -439,7 +439,6 @@ test_expect_success 'test --mixed <paths>' '
 '
 
 test_expect_success 'test resetting the index at give paths' '
-
 	mkdir sub &&
 	>sub/file1 &&
 	>sub/file2 &&
@@ -452,7 +451,6 @@ test_expect_success 'test resetting the index at give paths' '
 	echo "$U" &&
 	test_must_fail git diff-index --cached --exit-code "$T" &&
 	test "$T" != "$U"
-
 '
 
 test_expect_success 'resetting an unmodified path is a no-op' '
@@ -490,7 +488,6 @@ test_expect_success 'resetting specific path that is unmerged' '
 '
 
 test_expect_success 'disambiguation (1)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -499,11 +496,9 @@ test_expect_success 'disambiguation (1)' '
 	test -z "$(git diff --cached --name-only)" &&
 	test -f secondfile &&
 	test_must_be_empty secondfile
-
 '
 
 test_expect_success 'disambiguation (2)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -511,11 +506,9 @@ test_expect_success 'disambiguation (2)' '
 	test_must_fail git reset secondfile &&
 	test -n "$(git diff --cached --name-only -- secondfile)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (3)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -524,11 +517,9 @@ test_expect_success 'disambiguation (3)' '
 	test_must_fail git diff --quiet &&
 	test -z "$(git diff --cached --name-only)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (4)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index a800bda5e3..b527f8009c 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,6 @@ fill () {
 
 
 test_expect_success setup '
-
 	fill x y z > same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
@@ -63,7 +62,6 @@ test_expect_success setup '
 '
 
 test_expect_success 'checkout from non-existing branch' '
-
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
 	test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
@@ -72,7 +70,6 @@ test_expect_success 'checkout from non-existing branch' '
 '
 
 test_expect_success 'checkout with dirty tree without -m' '
-
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
 	then
@@ -81,11 +78,9 @@ test_expect_success 'checkout with dirty tree without -m' '
 	else
 		echo "happy - failed correctly"
 	fi
-
 '
 
 test_expect_success 'checkout with unrelated dirty tree without -m' '
-
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
 	cp same kept &&
@@ -96,7 +91,6 @@ test_expect_success 'checkout with unrelated dirty tree without -m' '
 '
 
 test_expect_success 'checkout -m with dirty tree' '
-
 	git checkout -f master &&
 	git clean -f &&
 
@@ -121,7 +115,6 @@ test_expect_success 'checkout -m with dirty tree' '
 '
 
 test_expect_success 'checkout -m with dirty tree, renamed' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 2 3 4 5 7 8 >one &&
@@ -139,11 +132,9 @@ test_expect_success 'checkout -m with dirty tree, renamed' '
 	! test -f one &&
 	git diff --cached >current &&
 	test_must_be_empty current
-
 '
 
 test_expect_success 'checkout -m with merge conflict' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 T 3 4 5 6 S 8 >one &&
@@ -166,7 +157,6 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-
 	git checkout -f master && git clean -f &&
 
 	fill b d > two &&
@@ -190,7 +180,6 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 
 	fill b d > two &&
@@ -216,7 +205,6 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 	git rm two &&
 
@@ -228,7 +216,6 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 '
 
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
-
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
 	git checkout -f renamer && git clean -f &&
@@ -267,7 +254,6 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-
 	git checkout -f master && git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -283,7 +269,6 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-
 	git checkout -f master && git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -299,7 +284,6 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-
 	git checkout -f master && git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -315,7 +299,6 @@ test_expect_success 'checkout to detach HEAD with HEAD^0' '
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git tag both side &&
 	git branch both master &&
 	git reset --hard &&
@@ -327,11 +310,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	test "z$H" = "z$M" &&
 	name=$(git symbolic-ref HEAD 2>/dev/null) &&
 	test "z$name" = zrefs/heads/both
-
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -351,11 +332,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	else
 		: happy
 	fi
-
 '
 
 test_expect_success 'switch branches while in subdirectory' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -366,11 +345,9 @@ test_expect_success 'switch branches while in subdirectory' '
 	) &&
 	! test -f subs/one &&
 	rm -fr subs
-
 '
 
 test_expect_success 'checkout specific path while in subdirectory' '
-
 	git reset --hard &&
 	git checkout side &&
 	mkdir subs &&
@@ -385,7 +362,6 @@ test_expect_success 'checkout specific path while in subdirectory' '
 		git checkout side -- bero
 	) &&
 	test -f subs/bero
-
 '
 
 test_expect_success 'checkout w/--track sets up tracking' '
@@ -608,7 +584,6 @@ test_expect_success 'failing checkout -b should not break working tree' '
 	test $(git symbolic-ref HEAD) = refs/heads/master &&
 	git diff --exit-code &&
 	git diff --cached --exit-code
-
 '
 
 test_expect_success 'switch out of non-branch' '
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v2 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
  2020-10-17  7:54   ` [PATCH v2 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
  2020-10-17  7:54   ` [PATCH v2 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
@ 2020-10-17  7:54   ` Charvi Mendiratta
  2020-10-17  7:54   ` [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells Charvi Mendiratta
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-17  7:54 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, sunshine, Charvi Mendiratta

According to Documentation/CodingGuidelines, there should be no whitespace after redirect operators. So, we should remove these whitespaces after redirect operators.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7102-reset.sh | 30 +++++++++++++++---------------
 t/t7201-co.sh    | 10 +++++-----
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 2b4cfb2c83..a8c96bf162 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -70,15 +70,15 @@ check_changes () {
 
 test_expect_success 'reset --hard message' '
 	hex=$(git log -1 --format="%h") &&
-	git reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg) > .expected &&
+	git reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
 test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' '
 	hex=$(git log -1 --format="%h") &&
-	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg $test_encoding) > .expected &&
+	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg $test_encoding) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
@@ -387,25 +387,25 @@ test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge
 '
 
 test_expect_success 'test --mixed <paths>' '
-	echo 1 > file1 &&
-	echo 2 > file2 &&
+	echo 1 >file1 &&
+	echo 2 >file2 &&
 	git add file1 file2 &&
 	test_tick &&
 	git commit -m files &&
 	before1=$(git rev-parse --short HEAD:file1) &&
 	before2=$(git rev-parse --short HEAD:file2) &&
 	git rm file2 &&
-	echo 3 > file3 &&
-	echo 4 > file4 &&
-	echo 5 > file1 &&
+	echo 3 >file3 &&
+	echo 4 >file4 &&
+	echo 5 >file1 &&
 	after1=$(git rev-parse --short $(git hash-object file1)) &&
 	after4=$(git rev-parse --short $(git hash-object file4)) &&
 	git add file1 file3 file4 &&
 	git reset HEAD -- file1 file2 file3 &&
 	test_must_fail git diff --quiet &&
-	git diff > output &&
+	git diff >output &&
 
-	cat > expect <<-EOF &&
+	cat >expect <<-EOF &&
 	diff --git a/file1 b/file1
 	index $before1..$after1 100644
 	--- a/file1
@@ -423,9 +423,9 @@ test_expect_success 'test --mixed <paths>' '
 	EOF
 
 	test_cmp expect output &&
-	git diff --cached > output &&
+	git diff --cached >output &&
 
-	cat > cached_expect <<-EOF &&
+	cat >cached_expect <<-EOF &&
 	diff --git a/file4 b/file4
 	new file mode 100644
 	index 0000000..$after4
@@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
 	git diff-index --cached --exit-code HEAD
 '
 
-cat > expect << EOF
+cat >expect << EOF
 Unstaged changes after reset:
 M	file2
 EOF
 
 test_expect_success '--mixed refreshes the index' '
 	echo 123 >> file2 &&
-	git reset --mixed HEAD > output &&
+	git reset --mixed HEAD >output &&
 	test_i18ncmp expect output
 '
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index b527f8009c..74553f991b 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,7 @@ fill () {
 
 
 test_expect_success setup '
-	fill x y z > same &&
+	fill x y z >same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
 	git add same one two &&
@@ -55,7 +55,7 @@ test_expect_success setup '
 
 	git checkout -b simple master &&
 	rm -f one &&
-	fill a c e > two &&
+	fill a c e >two &&
 	git commit -a -m "Simple D one, M two" &&
 
 	git checkout master
@@ -95,7 +95,7 @@ test_expect_success 'checkout -m with dirty tree' '
 	git clean -f &&
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
-	git checkout -m side > messages &&
+	git checkout -m side >messages &&
 
 	test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
 
@@ -159,7 +159,7 @@ test_expect_success 'checkout -m with merge conflict' '
 test_expect_success 'format of merge conflict from checkout -m' '
 	git checkout -f master && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout -m simple &&
 
 	git ls-files >current &&
@@ -182,7 +182,7 @@ test_expect_success 'format of merge conflict from checkout -m' '
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 	git checkout -f master && git reset --hard && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
 
 	cat <<-EOF >expect &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
                     ` (2 preceding siblings ...)
  2020-10-17  7:54   ` [PATCH v2 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
@ 2020-10-17  7:54   ` Charvi Mendiratta
  2020-10-18 15:39     ` Phillip Wood
  2020-10-17  7:54   ` [PATCH v2 5/5][Outreachy] t7201: place each command in its own line Charvi Mendiratta
                     ` (2 subsequent siblings)
  6 siblings, 1 reply; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-17  7:54 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, sunshine, Charvi Mendiratta

Avoid using `cd` outside of subshells since, if the test fails, there is no guarantee that the current working directory is the expected one, which may cause subsequent tests to run in the wrong directory.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 74553f991b..5898182fd2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
 	git checkout master &&
 
 	mkdir subs &&
-	(
-		cd subs &&
-		git checkout side
-	) &&
+	git -C subs checkout side &&
 	! test -f subs/one &&
 	rm -fr subs
 '
@@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 	git checkout master &&
 	mkdir -p subs &&
-	(
-		cd subs &&
-		git checkout side -- bero
-	) &&
+	git -C subs checkout side -- bero &&
 	test -f subs/bero
 '
 
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v2 5/5][Outreachy] t7201: place each command in its own line
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
                     ` (3 preceding siblings ...)
  2020-10-17  7:54   ` [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells Charvi Mendiratta
@ 2020-10-17  7:54   ` Charvi Mendiratta
  2020-10-20 11:43   ` [PATCH v3 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
  2020-10-20 12:11   ` [PATCH v4] t7201: put each command on a separate line Charvi Mendiratta
  6 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-17  7:54 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, sunshine, Charvi Mendiratta

Multiple commands on one line should be split across multiple lines.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5898182fd2..b36a93056f 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -157,7 +157,8 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout -m simple &&
@@ -180,7 +181,9 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
@@ -205,7 +208,9 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 	git rm two &&
 
 	test_must_fail git checkout simple 2>errs &&
@@ -218,7 +223,8 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	git checkout renamer^ 2>messages &&
 	test_i18ngrep "HEAD is now at $rev" messages &&
 	test_line_count = 1 messages &&
@@ -237,7 +243,8 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '
 test_expect_success 'checkout to detach HEAD' '
 	git config advice.detachedHead true &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
 	grep "HEAD is now at $rev" messages &&
 	test_line_count -gt 1 messages &&
@@ -254,7 +261,8 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -269,7 +277,8 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -284,7 +293,8 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH 0/5][Outreachy] modernizing the test scripts
  2020-10-16 12:54 ` [PATCH 0/5][Outreachy] modernizing the test scripts Christian Couder
@ 2020-10-17  8:27   ` Charvi Mendiratta
  0 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-17  8:27 UTC (permalink / raw)
  To: Christian Couder; +Cc: git, phillip.wood123, Eric Sunshine

On Fri, 16 Oct 2020 at 18:24, Christian Couder
<christian.couder@gmail.com> wrote:

Thank you Christian, I have sent the new updated patch series .
>
> On Thu, Oct 15, 2020 at 7:57 PM charvi-077 <charvi077@gmail.com> wrote:
> >
> > This is my first patch series to the GIT mailing list. I followed the
> > link[1] and t7001 patches to modernize and clean up the test scripts.
>
> Thanks for getting started contributing!
>
> > This patch series :
> >  -modernize the three test scripts : t7101, t7201 and t102.
>
> s/t102/t7102/
>
> On https://git.github.io/Outreachy-21-Microprojects/ we say:
>
> "Find one test script that needs some of the same changes and make them."
>
> So working only one test script, for example only t7101, would have
> been better than working on 3 test scripts. Now that you started
> working on 3 test scripts, it's ok to finish modernizing all these 3
> test scripts though.
>
Yes, I agree this but t7101 has very minor changes required that I
have completed so I switched to another one simultaneously . But in
future I will make sure to follow this practise . Also, till now I
have done majority of the changes as mentioned in the link in all the
three scripts and will try to do more and complete it .

> >  - cleans up with 5 types of changes in all the three scripts.
> >    1. Converting the old old style test format to new one
>
> s/old old/old/
>
> >    2. Removing blankspaces in test bodies
> >    3. Removing whitespaces after the redirect operator, according to
> >       Codingguidelines .
>
> s/Codingguidelines/CodingGuidelines/
>
> >    4. Using git -C instead of cd
> >    5. Placing all commands in seperate lines.
>
> s/seperate/separate/
>
> > Also, I have tested the scripts and set up travis CI[2].
>
> Nice!
>
> Thanks,
> Christian.

Thanks and Regards,
Charvi

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body
  2020-10-17  7:54   ` [PATCH v2 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
@ 2020-10-17 15:13     ` Đoàn Trần Công Danh
  2020-10-18  5:40       ` Charvi Mendiratta
  0 siblings, 1 reply; 60+ messages in thread
From: Đoàn Trần Công Danh @ 2020-10-17 15:13 UTC (permalink / raw)
  To: Charvi Mendiratta; +Cc: git, gitster, christian.couder, sunshine

On 2020-10-17 13:24:52+0530, Charvi Mendiratta <charvi077@gmail.com> wrote:

Welcome to the list.

> Some tests use a deprecated style in which there are unnecessary blank lines after the opening quote of the test body and before the closing quote. So we should remove these unnecessary blank lines.

In Git project, we wrap the commit message's body to 72 columns per
line (for more information, please take a look at
Documentation/MyFirstContribution.txt).

And we rarely say "we should", if the change shouldn't be applied,
it won't be applied.
Instead, we ask the code base to fix itself. Perhaps:

	t7102 and t7201 still follow the old style of having blank
	lines around test body, which is not consistence with our
	current practice.

	Let's remove those unnecessary blank lines.

Thanks,
-- 
Danh

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body
  2020-10-17 15:13     ` Đoàn Trần Công Danh
@ 2020-10-18  5:40       ` Charvi Mendiratta
  0 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-18  5:40 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: git, Junio C Hamano, Christian Couder, Eric Sunshine

On Sat, 17 Oct 2020 at 20:44, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
>
> On 2020-10-17 13:24:52+0530, Charvi Mendiratta <charvi077@gmail.com> wrote:
>
> Welcome to the list.
>
> > Some tests use a deprecated style in which there are unnecessary blank lines after the opening quote of the test body and before the closing quote. So we should remove these unnecessary blank lines.
>
> In Git project, we wrap the commit message's body to 72 columns per
> line (for more information, please take a look at
> Documentation/MyFirstContribution.txt).
>

Thanks a lot Danh, I will fix it in my editor's settings and will
update in the next patch series .

> And we rarely say "we should", if the change shouldn't be applied,
> it won't be applied.
> Instead, we ask the code base to fix itself. Perhaps:
>
>         t7102 and t7201 still follow the old style of having blank
>         lines around test body, which is not consistence with our
>         current practice.
>
>         Let's remove those unnecessary blank lines.
>

Noted, will update this as well.

> Thanks,
> --
> Danh

Thanks and Regards,
Charvi

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-17  7:54   ` [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells Charvi Mendiratta
@ 2020-10-18 15:39     ` Phillip Wood
  2020-10-19 12:55       ` Charvi Mendiratta
  0 siblings, 1 reply; 60+ messages in thread
From: Phillip Wood @ 2020-10-18 15:39 UTC (permalink / raw)
  To: Charvi Mendiratta, git; +Cc: gitster, christian.couder, sunshine

Hi Charvi

Congratulations on posting your first patch series.

On 17/10/2020 08:54, Charvi Mendiratta wrote:
> Avoid using `cd` outside of subshells since, if the test fails, there is no guarantee that the current working directory is the expected one, which may cause subsequent tests to run in the wrong directory.

That is an accurate description of why we want to avoid using `cd` 
outside of subshells. However this conversion is converting `cd` inside 
a subshell to use `git -C`. I think that is worthwhile as it avoids 
having to use a subshell but the description should say explain that the 
conversion is desirable to avoid the cost of starting a subshell as the 
original test does not suffer from the problem described in your commit 
message.

Best Wishes

Phillip

> 
> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> ---
>   t/t7201-co.sh | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 74553f991b..5898182fd2 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
>   	git checkout master &&
>   
>   	mkdir subs &&
> -	(
> -		cd subs &&
> -		git checkout side
> -	) &&
> +	git -C subs checkout side &&
>   	! test -f subs/one &&
>   	rm -fr subs
>   '
> @@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
>   
>   	git checkout master &&
>   	mkdir -p subs &&
> -	(
> -		cd subs &&
> -		git checkout side -- bero
> -	) &&
> +	git -C subs checkout side -- bero &&
>   	test -f subs/bero
>   '
>   
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-18 15:39     ` Phillip Wood
@ 2020-10-19 12:55       ` Charvi Mendiratta
  2020-10-19 13:46         ` Phillip Wood
  0 siblings, 1 reply; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-19 12:55 UTC (permalink / raw)
  To: phillip.wood; +Cc: git, Junio C Hamano, Christian Couder, Eric Sunshine

On Sun, 18 Oct 2020 at 21:09, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Charvi
>
> Congratulations on posting your first patch series.
>
> On 17/10/2020 08:54, Charvi Mendiratta wrote:
> > Avoid using `cd` outside of subshells since, if the test fails, there is no guarantee that the current working directory is the expected one, which may cause subsequent tests to run in the wrong directory.
>
> That is an accurate description of why we want to avoid using `cd`
> outside of subshells. However this conversion is converting `cd` inside
> a subshell to use `git -C`. I think that is worthwhile as it avoids
> having to use a subshell but the description should say explain that the
> conversion is desirable to avoid the cost of starting a subshell as the
> original test does not suffer from the problem described in your commit
> message.
>

Thank you Philip, for corrections . I somewhat able to understand that
commit message
should be " avoid using cd inside the subshells " because running a
shell script itselfs starts
a new subshell, please correct me if I am wrong . But still I am
unable to get that why you
mentioned the description as "cost of starting a new subshell " . Will
this not be the same subshell ?

> Best Wishes
>
> Phillip
>
> >
> > Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> > ---
> >   t/t7201-co.sh | 10 ++--------
> >   1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> > index 74553f991b..5898182fd2 100755
> > --- a/t/t7201-co.sh
> > +++ b/t/t7201-co.sh
> > @@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
> >       git checkout master &&
> >
> >       mkdir subs &&
> > -     (
> > -             cd subs &&
> > -             git checkout side
> > -     ) &&

Is there any specific meaning of writing these above two commands in
parentheses . Will this not work the same without it ?

> > +     git -C subs checkout side &&
> >       ! test -f subs/one &&
> >       rm -fr subs
> >   '
> > @@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
> >
> >       git checkout master &&
> >       mkdir -p subs &&
> > -     (
> > -             cd subs &&
> > -             git checkout side -- bero
> > -     ) &&
> > +     git -C subs checkout side -- bero &&
> >       test -f subs/bero
> >   '
> >
> >
Thanks and Regards ,
Charvi

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-19 12:55       ` Charvi Mendiratta
@ 2020-10-19 13:46         ` Phillip Wood
  2020-10-19 17:24           ` Charvi Mendiratta
  0 siblings, 1 reply; 60+ messages in thread
From: Phillip Wood @ 2020-10-19 13:46 UTC (permalink / raw)
  To: Charvi Mendiratta, phillip.wood
  Cc: git, Junio C Hamano, Christian Couder, Eric Sunshine

Hi Charvi

On 19/10/2020 13:55, Charvi Mendiratta wrote:
> On Sun, 18 Oct 2020 at 21:09, Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> Hi Charvi
>>
>> Congratulations on posting your first patch series.
>>
>> On 17/10/2020 08:54, Charvi Mendiratta wrote:
>>> Avoid using `cd` outside of subshells since, if the test fails, there is no guarantee that the current working directory is the expected one, which may cause subsequent tests to run in the wrong directory.
>>
>> That is an accurate description of why we want to avoid using `cd`
>> outside of subshells. However this conversion is converting `cd` inside
>> a subshell to use `git -C`. I think that is worthwhile as it avoids
>> having to use a subshell but the description should say explain that the
>> conversion is desirable to avoid the cost of starting a subshell as the
>> original test does not suffer from the problem described in your commit
>> message.
>>
> 
> Thank you Philip, for corrections . I somewhat able to understand that
> commit message
> should be " avoid using cd inside the subshells " because running a
> shell script itselfs starts
> a new subshell, please correct me if I am wrong . But still I am
> unable to get that why you
> mentioned the description as "cost of starting a new subshell " . Will
> this not be the same subshell ?

The original test looks something like
(
	cd sub &&
	git something
) &&

The commands between the ( and ) are executed in a subshell, any changes 
made to the current directory or shell variables in the subshell do not 
affect the rest of the test script. This is because the subshell starts 
a separate shell process, but creating this separate process has a cost 
associated with it.

The modified test looks like
	git -C sub something

Here we tell git to change directory before it runs the 'something' 
command, this is more efficient as we don't need to start any extra 
processes - there are no subshells.

So the purpose of this change is not to "avoid using cd inside a 
subshell" but to avoid having to use a subshell at all.

I hope that helps explain what a subshell is and why we want to avoid 
using it if we can, do let me know if you want me to clarify anything.

Best Wishes

Phillip


>> Best Wishes
>>
>> Phillip
>>
>>>
>>> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
>>> ---
>>>    t/t7201-co.sh | 10 ++--------
>>>    1 file changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
>>> index 74553f991b..5898182fd2 100755
>>> --- a/t/t7201-co.sh
>>> +++ b/t/t7201-co.sh
>>> @@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
>>>        git checkout master &&
>>>
>>>        mkdir subs &&
>>> -     (
>>> -             cd subs &&
>>> -             git checkout side
>>> -     ) &&
> 
> Is there any specific meaning of writing these above two commands in
> parentheses . Will this not work the same without it ?
> 
>>> +     git -C subs checkout side &&
>>>        ! test -f subs/one &&
>>>        rm -fr subs
>>>    '
>>> @@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
>>>
>>>        git checkout master &&
>>>        mkdir -p subs &&
>>> -     (
>>> -             cd subs &&
>>> -             git checkout side -- bero
>>> -     ) &&
>>> +     git -C subs checkout side -- bero &&
>>>        test -f subs/bero
>>>    '
>>>
>>>
> Thanks and Regards ,
> Charvi
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-19 13:46         ` Phillip Wood
@ 2020-10-19 17:24           ` Charvi Mendiratta
  2020-10-19 20:25             ` Taylor Blau
  0 siblings, 1 reply; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-19 17:24 UTC (permalink / raw)
  To: phillip.wood; +Cc: git, Junio C Hamano, Christian Couder, Eric Sunshine

On Mon, 19 Oct 2020 at 19:16, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Charvi
>
> On 19/10/2020 13:55, Charvi Mendiratta wrote:
> > On Sun, 18 Oct 2020 at 21:09, Phillip Wood <phillip.wood123@gmail.com> wrote:
> >>
> >> Hi Charvi
> >>
> >> Congratulations on posting your first patch series.
> >>
> >> On 17/10/2020 08:54, Charvi Mendiratta wrote:
> >>> Avoid using `cd` outside of subshells since, if the test fails, there is no guarantee that the current working directory is the expected one, which may cause subsequent tests to run in the wrong directory.
> >>
> >> That is an accurate description of why we want to avoid using `cd`
> >> outside of subshells. However this conversion is converting `cd` inside
> >> a subshell to use `git -C`. I think that is worthwhile as it avoids
> >> having to use a subshell but the description should say explain that the
> >> conversion is desirable to avoid the cost of starting a subshell as the
> >> original test does not suffer from the problem described in your commit
> >> message.
> >>
> >
> > Thank you Philip, for corrections . I somewhat able to understand that
> > commit message
> > should be " avoid using cd inside the subshells " because running a
> > shell script itselfs starts
> > a new subshell, please correct me if I am wrong . But still I am
> > unable to get that why you
> > mentioned the description as "cost of starting a new subshell " . Will
> > this not be the same subshell ?
>
> The original test looks something like
> (
>         cd sub &&
>         git something
> ) &&
>
> The commands between the ( and ) are executed in a subshell, any changes
> made to the current directory or shell variables in the subshell do not
> affect the rest of the test script. This is because the subshell starts
> a separate shell process, but creating this separate process has a cost
> associated with it.
>
> The modified test looks like
>         git -C sub something
>
> Here we tell git to change directory before it runs the 'something'
> command, this is more efficient as we don't need to start any extra
> processes - there are no subshells.
>
> So the purpose of this change is not to "avoid using cd inside a
> subshell" but to avoid having to use a subshell at all.
>
> I hope that helps explain what a subshell is and why we want to avoid
> using it if we can, do let me know if you want me to clarify anything.
>

Yes, thanks a lot Philip I understood the reason. I will do the corrections in
commit message and commit body as below :
t7201: using 'git -C' to avoid subshell

Using 'git-C' instead of 'cd' inside of subshell, to avoid the extra process
of starting a new subshell

Please confirm, if any other changes are required .

> Best Wishes
>
> Phillip
>
Thanks and Regards,
Charvi
>
> >> Best Wishes
> >>
> >> Phillip
> >>
> >>>
> >>> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> >>> ---
> >>>    t/t7201-co.sh | 10 ++--------
> >>>    1 file changed, 2 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> >>> index 74553f991b..5898182fd2 100755
> >>> --- a/t/t7201-co.sh
> >>> +++ b/t/t7201-co.sh
> >>> @@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
> >>>        git checkout master &&
> >>>
> >>>        mkdir subs &&
> >>> -     (
> >>> -             cd subs &&
> >>> -             git checkout side
> >>> -     ) &&
> >
> > Is there any specific meaning of writing these above two commands in
> > parentheses . Will this not work the same without it ?
> >
> >>> +     git -C subs checkout side &&
> >>>        ! test -f subs/one &&
> >>>        rm -fr subs
> >>>    '
> >>> @@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
> >>>
> >>>        git checkout master &&
> >>>        mkdir -p subs &&
> >>> -     (
> >>> -             cd subs &&
> >>> -             git checkout side -- bero
> >>> -     ) &&
> >>> +     git -C subs checkout side -- bero &&
> >>>        test -f subs/bero
> >>>    '
> >>>
> >>>
> > Thanks and Regards ,
> > Charvi
> >

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-19 17:24           ` Charvi Mendiratta
@ 2020-10-19 20:25             ` Taylor Blau
  2020-10-20  5:38               ` Charvi Mendiratta
  2020-10-20  9:13               ` Phillip Wood
  0 siblings, 2 replies; 60+ messages in thread
From: Taylor Blau @ 2020-10-19 20:25 UTC (permalink / raw)
  To: Charvi Mendiratta
  Cc: phillip.wood, git, Junio C Hamano, Christian Couder, Eric Sunshine

Hi Charvi,

On Mon, Oct 19, 2020 at 10:54:57PM +0530, Charvi Mendiratta wrote:
> Yes, thanks a lot Philip I understood the reason. I will do the corrections in
> commit message and commit body as below :
> t7201: using 'git -C' to avoid subshell
>
> Using 'git-C' instead of 'cd' inside of subshell, to avoid the extra process
> of starting a new subshell
>
> Please confirm, if any other changes are required .

Usually it never hurts to just send the patch, since any feedback that a
reviewer has now is equally good even after you have sent a patch. Plus,
it's easier to review the concrete patch you want applied, instead of a
hypothetical of what you might send.

That said, a couple of notes:

  - Your subject message is good. It is concise, to-the-point, and
    accurately describes the change. Good.

  - The body is similarly short, but could be rewritten to use the
    imperative mood. But, it is redundant with the subject. The subject
    says "we are using 'git -C' to avoid creating a subshell", and the
    patch says exactly the same.

...So, you can do one of two things. Either you can abbreviate the
subject, adding the additional detail in the patch message, or you could
leave the subject as-is and delete the patch message entirely.

Either would be fine with me, but certainly Phillip or others could
chime in, too.

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-19 20:25             ` Taylor Blau
@ 2020-10-20  5:38               ` Charvi Mendiratta
  2020-10-20 20:09                 ` Taylor Blau
  2020-10-20  9:13               ` Phillip Wood
  1 sibling, 1 reply; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20  5:38 UTC (permalink / raw)
  To: Taylor Blau
  Cc: phillip.wood, git, Junio C Hamano, Christian Couder, Eric Sunshine

On Tue, 20 Oct 2020 at 01:55, Taylor Blau <me@ttaylorr.com> wrote:
>
> Hi Charvi,
>
> On Mon, Oct 19, 2020 at 10:54:57PM +0530, Charvi Mendiratta wrote:
> > Yes, thanks a lot Philip I understood the reason. I will do the corrections in
> > commit message and commit body as below :
> > t7201: using 'git -C' to avoid subshell
> >
> > Using 'git-C' instead of 'cd' inside of subshell, to avoid the extra process
> > of starting a new subshell
> >
> > Please confirm, if any other changes are required .
>
> Usually it never hurts to just send the patch, since any feedback that a
> reviewer has now is equally good even after you have sent a patch. Plus,
> it's easier to review the concrete patch you want applied, instead of a
> hypothetical of what you might send.
>

Yes, I completely agree with you . Its my fault, I will send it in the
patch and will
take care of not repeating this again .

> That said, a couple of notes:
>
>   - Your subject message is good. It is concise, to-the-point, and
>     accurately describes the change. Good.
>
>   - The body is similarly short, but could be rewritten to use the
>     imperative mood. But, it is redundant with the subject. The subject
>     says "we are using 'git -C' to avoid creating a subshell", and the
>     patch says exactly the same.
>
> ...So, you can do one of two things. Either you can abbreviate the
> subject, adding the additional detail in the patch message, or you could
> leave the subject as-is and delete the patch message entirely.
>
Thanks Taylor, I will do the changes as you mentioned and send it in the
next patch .

> Either would be fine with me, but certainly Phillip or others could
> chime in, too.
>
> Thanks,
> Taylor

Thanks and Regards,
Charvi

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-19 20:25             ` Taylor Blau
  2020-10-20  5:38               ` Charvi Mendiratta
@ 2020-10-20  9:13               ` Phillip Wood
  2020-10-20 11:48                 ` Charvi Mendiratta
  1 sibling, 1 reply; 60+ messages in thread
From: Phillip Wood @ 2020-10-20  9:13 UTC (permalink / raw)
  To: Taylor Blau, Charvi Mendiratta
  Cc: phillip.wood, git, Junio C Hamano, Christian Couder, Eric Sunshine

Hi Charvi

On 19/10/2020 21:25, Taylor Blau wrote:
> Hi Charvi,
> 
> On Mon, Oct 19, 2020 at 10:54:57PM +0530, Charvi Mendiratta wrote:
>> Yes, thanks a lot Philip I understood the reason. I will do the corrections in
>> commit message and commit body as below :
>> t7201: using 'git -C' to avoid subshell
>>
>> Using 'git-C' instead of 'cd' inside of subshell, to avoid the extra process
>> of starting a new subshell
>>
>> [...]
> 
> That said, a couple of notes:
> 
>    - Your subject message is good. It is concise, to-the-point, and
>      accurately describes the change. Good.
> 
>    - The body is similarly short, but could be rewritten to use the
>      imperative mood. But, it is redundant with the subject. The subject
>      says "we are using 'git -C' to avoid creating a subshell", and the
>      patch says exactly the same.
> 
> ...So, you can do one of two things. Either you can abbreviate the
> subject, adding the additional detail in the patch message, or you could
> leave the subject as-is and delete the patch message entirely.
> 
> Either would be fine with me, but certainly Phillip or others could
> chime in, too.

I'm happy with either, but I would suggest changing the subject so the 
description starts with 'use' rather than 'using'

Best Wishes

Phillip

> Thanks,
> Taylor
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 0/5][Outreachy] modernize the test scripts
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
                     ` (4 preceding siblings ...)
  2020-10-17  7:54   ` [PATCH v2 5/5][Outreachy] t7201: place each command in its own line Charvi Mendiratta
@ 2020-10-20 11:43   ` Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
                       ` (4 more replies)
  2020-10-20 12:11   ` [PATCH v4] t7201: put each command on a separate line Charvi Mendiratta
  6 siblings, 5 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20 11:43 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, phillip.wood123, congdanhqx, me,
	Charvi Mendiratta

I thanks to the community for reviewing my patches, in this patch 
series I have updated my commits messages and body as suggested 
by the community.  

This patch series :
 -modernize the three test scripts : t7101, t7201 and t7102.
 - cleans up with 5 types of changes in all the three scripts.
   1. Converting the old style test format to new one
   2. Removing blankspaces in test bodies
   3. Removing whitespaces after the redirect operator, according to
      CodingGuidelines 
   4. Using 'git -C' to avoid use of another subshell 
   5. Placing commands in separate lines

Charvi Mendiratta (5):
  t7101,t7102,t7201: modernize test formatting
  t7102,t7201: remove unnecessary blank spaces in test body
  t7102,t7201: remove whitespace after redirect operator
  t7201: use 'git -C' to avoid subshell
  t7201: put each command on a seperate line

 t/t7101-reset-empty-subdirs.sh |  66 ++++++++++-----------
 t/t7102-reset.sh               |  63 ++++++++------------
 t/t7201-co.sh                  | 102 +++++++++++++--------------------
 3 files changed, 96 insertions(+), 135 deletions(-)

-- 
2.29.0.rc1


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v3 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting
  2020-10-20 11:43   ` [PATCH v3 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
@ 2020-10-20 11:43     ` Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20 11:43 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, phillip.wood123, congdanhqx, me,
	Charvi Mendiratta

Some tests in these scripts are formatted using a very old style:
        test_expect_success \
            'title' \
            'body line 1 &&
             body line 2'

Updating the formatting to the modern style:
        test_expect_success 'title' '
            body line 1 &&
            body line 2
        '

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7101-reset-empty-subdirs.sh | 66 +++++++++++++++++-----------------
 t/t7102-reset.sh               | 24 +++++--------
 t/t7201-co.sh                  | 31 ++++++++--------
 3 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/t/t7101-reset-empty-subdirs.sh b/t/t7101-reset-empty-subdirs.sh
index 96e163f084..bfce05ac5d 100755
--- a/t/t7101-reset-empty-subdirs.sh
+++ b/t/t7101-reset-empty-subdirs.sh
@@ -6,16 +6,15 @@
 test_description='git reset should cull empty subdirs'
 . ./test-lib.sh
 
-test_expect_success \
-    'creating initial files' \
-    'mkdir path0 &&
+test_expect_success 'creating initial files' '
+     mkdir path0 &&
      cp "$TEST_DIRECTORY"/../COPYING path0/COPYING &&
      git add path0/COPYING &&
-     git commit -m add -a'
+     git commit -m add -a
+'
 
-test_expect_success \
-    'creating second files' \
-    'mkdir path1 &&
+test_expect_success 'creating second files' '
+     mkdir path1 &&
      mkdir path1/path2 &&
      cp "$TEST_DIRECTORY"/../COPYING path1/path2/COPYING &&
      cp "$TEST_DIRECTORY"/../COPYING path1/COPYING &&
@@ -25,39 +24,40 @@ test_expect_success \
      git add path1/COPYING &&
      git add COPYING &&
      git add path0/COPYING-TOO &&
-     git commit -m change -a'
+     git commit -m change -a
+'
 
-test_expect_success \
-    'resetting tree HEAD^' \
-    'git reset --hard HEAD^'
+test_expect_success 'resetting tree HEAD^' '
+     git reset --hard HEAD^
+'
 
-test_expect_success \
-    'checking initial files exist after rewind' \
-    'test -d path0 &&
-     test -f path0/COPYING'
+test_expect_success 'checking initial files exist after rewind' '
+     test -d path0 &&
+     test -f path0/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/path2/COPYING' \
-    '! test -f path1/path2/COPYING'
+test_expect_success 'checking lack of path1/path2/COPYING' '
+    ! test -f path1/path2/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/COPYING' \
-    '! test -f path1/COPYING'
+test_expect_success 'checking lack of path1/COPYING' '
+    ! test -f path1/COPYING
+'
 
-test_expect_success \
-    'checking lack of COPYING' \
-    '! test -f COPYING'
+test_expect_success 'checking lack of COPYING' '
+     ! test -f COPYING
+'
 
-test_expect_success \
-    'checking checking lack of path1/COPYING-TOO' \
-    '! test -f path0/COPYING-TOO'
+test_expect_success 'checking checking lack of path1/COPYING-TOO' '
+     ! test -f path0/COPYING-TOO
+'
 
-test_expect_success \
-    'checking lack of path1/path2' \
-    '! test -d path1/path2'
+test_expect_success 'checking lack of path1/path2' '
+     ! test -d path1/path2
+'
 
-test_expect_success \
-    'checking lack of path1' \
-    '! test -d path1'
+test_expect_success 'checking lack of path1' '
+     ! test -d path1
+'
 
 test_done
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 22161b3b2d..fe43f77513 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -107,8 +107,7 @@ test_expect_success 'reset --soft with unmerged index should fail' '
 	git rm --cached -- un
 '
 
-test_expect_success \
-	'giving paths with options different than --mixed should fail' '
+test_expect_success 'giving paths with options different than --mixed should fail' '
 	test_must_fail git reset --soft -- first &&
 	test_must_fail git reset --hard -- first &&
 	test_must_fail git reset --soft HEAD^ -- first &&
@@ -128,8 +127,7 @@ test_expect_success 'giving unrecognized options should fail' '
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending merge should fail' '
+test_expect_success 'trying to do reset --soft with pending merge should fail' '
 	git branch branch1 &&
 	git branch branch2 &&
 
@@ -152,8 +150,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending checkout merge should fail' '
+test_expect_success 'trying to do reset --soft with pending checkout merge should fail' '
 	git branch branch3 &&
 	git branch branch4 &&
 
@@ -175,8 +172,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'resetting to HEAD with no changes should succeed and do nothing' '
+test_expect_success 'resetting to HEAD with no changes should succeed and do nothing' '
 	git reset --hard &&
 		check_changes $head5 &&
 	git reset --hard HEAD &&
@@ -226,8 +222,7 @@ secondfile:
 2nd line 2nd file
 3rd line 2nd file
 EOF
-test_expect_success \
-	'changing files and redo the last commit should succeed' '
+test_expect_success 'changing files and redo the last commit should succeed' '
 	echo "3rd line 2nd file" >>secondfile &&
 	git commit -a -C ORIG_HEAD &&
 	head4=$(git rev-parse --verify HEAD) &&
@@ -245,8 +240,7 @@ first:
 second:
 2nd file
 EOF
-test_expect_success \
-	'--hard reset should change the files and undo commits permanently' '
+test_expect_success '--hard reset should change the files and undo commits permanently' '
 	git reset --hard HEAD~2 &&
 	check_changes $head5p2 &&
 	test "$(git rev-parse ORIG_HEAD)" = \
@@ -284,8 +278,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'redoing changes adding them without commit them should succeed' '
+test_expect_success 'redoing changes adding them without commit them should succeed' '
 	git rm first &&
 	git mv second secondfile &&
 
@@ -380,8 +373,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'--hard reset to ORIG_HEAD should clear a fast-forward merge' '
+test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge' '
 	git reset --hard HEAD^ &&
 	check_changes $head5 &&
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 4d62b9b00f..a800bda5e3 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -62,7 +62,7 @@ test_expect_success setup '
 	git checkout master
 '
 
-test_expect_success "checkout from non-existing branch" '
+test_expect_success 'checkout from non-existing branch' '
 
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
@@ -71,7 +71,7 @@ test_expect_success "checkout from non-existing branch" '
 	test refs/heads/master = "$(git symbolic-ref HEAD)"
 '
 
-test_expect_success "checkout with dirty tree without -m" '
+test_expect_success 'checkout with dirty tree without -m' '
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
@@ -84,7 +84,7 @@ test_expect_success "checkout with dirty tree without -m" '
 
 '
 
-test_expect_success "checkout with unrelated dirty tree without -m" '
+test_expect_success 'checkout with unrelated dirty tree without -m' '
 
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
@@ -95,7 +95,7 @@ test_expect_success "checkout with unrelated dirty tree without -m" '
 	test_cmp messages.expect messages
 '
 
-test_expect_success "checkout -m with dirty tree" '
+test_expect_success 'checkout -m with dirty tree' '
 
 	git checkout -f master &&
 	git clean -f &&
@@ -120,7 +120,7 @@ test_expect_success "checkout -m with dirty tree" '
 	test_must_be_empty current.index
 '
 
-test_expect_success "checkout -m with dirty tree, renamed" '
+test_expect_success 'checkout -m with dirty tree, renamed' '
 
 	git checkout -f master && git clean -f &&
 
@@ -388,22 +388,22 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 '
 
-test_expect_success \
-    'checkout w/--track sets up tracking' '
+test_expect_success 'checkout w/--track sets up tracking' '
     git config branch.autosetupmerge false &&
     git checkout master &&
     git checkout --track -b track1 &&
     test "$(git config branch.track1.remote)" &&
-    test "$(git config branch.track1.merge)"'
+    test "$(git config branch.track1.merge)"
+'
 
-test_expect_success \
-    'checkout w/autosetupmerge=always sets up tracking' '
+test_expect_success 'checkout w/autosetupmerge=always sets up tracking' '
     test_when_finished git config branch.autosetupmerge false &&
     git config branch.autosetupmerge always &&
     git checkout master &&
     git checkout -b track2 &&
     test "$(git config branch.track2.remote)" &&
-    test "$(git config branch.track2.merge)"'
+    test "$(git config branch.track2.merge)"
+'
 
 test_expect_success 'checkout w/--track from non-branch HEAD fails' '
     git checkout master^0 &&
@@ -435,8 +435,7 @@ test_expect_success 'detach a symbolic link HEAD' '
     test "z$(git rev-parse --verify refs/heads/master)" = "z$here"
 '
 
-test_expect_success \
-    'checkout with --track fakes a sensible -b <name>' '
+test_expect_success 'checkout with --track fakes a sensible -b <name>' '
     git config remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" &&
     git update-ref refs/remotes/origin/koala/bear renamer &&
 
@@ -457,9 +456,9 @@ test_expect_success \
     test "$(git rev-parse HEAD)" = "$(git rev-parse renamer)"
 '
 
-test_expect_success \
-    'checkout with --track, but without -b, fails with too short tracked name' '
-    test_must_fail git checkout --track renamer'
+test_expect_success 'checkout with --track, but without -b, fails with too short tracked name' '
+    test_must_fail git checkout --track renamer
+'
 
 setup_conflicting_index () {
 	rm -f .git/index &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body
  2020-10-20 11:43   ` [PATCH v3 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
@ 2020-10-20 11:43     ` Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20 11:43 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, phillip.wood123, congdanhqx, me,
	Charvi Mendiratta

t7102 and t7201 still follow the old style of having blank
lines around test body, which is not consistence with our
current practice.

Let's remove those unnecessary blank lines.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7102-reset.sh |  9 ---------
 t/t7201-co.sh    | 25 -------------------------
 2 files changed, 34 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index fe43f77513..2b4cfb2c83 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -439,7 +439,6 @@ test_expect_success 'test --mixed <paths>' '
 '
 
 test_expect_success 'test resetting the index at give paths' '
-
 	mkdir sub &&
 	>sub/file1 &&
 	>sub/file2 &&
@@ -452,7 +451,6 @@ test_expect_success 'test resetting the index at give paths' '
 	echo "$U" &&
 	test_must_fail git diff-index --cached --exit-code "$T" &&
 	test "$T" != "$U"
-
 '
 
 test_expect_success 'resetting an unmodified path is a no-op' '
@@ -490,7 +488,6 @@ test_expect_success 'resetting specific path that is unmerged' '
 '
 
 test_expect_success 'disambiguation (1)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -499,11 +496,9 @@ test_expect_success 'disambiguation (1)' '
 	test -z "$(git diff --cached --name-only)" &&
 	test -f secondfile &&
 	test_must_be_empty secondfile
-
 '
 
 test_expect_success 'disambiguation (2)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -511,11 +506,9 @@ test_expect_success 'disambiguation (2)' '
 	test_must_fail git reset secondfile &&
 	test -n "$(git diff --cached --name-only -- secondfile)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (3)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -524,11 +517,9 @@ test_expect_success 'disambiguation (3)' '
 	test_must_fail git diff --quiet &&
 	test -z "$(git diff --cached --name-only)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (4)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index a800bda5e3..b527f8009c 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,6 @@ fill () {
 
 
 test_expect_success setup '
-
 	fill x y z > same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
@@ -63,7 +62,6 @@ test_expect_success setup '
 '
 
 test_expect_success 'checkout from non-existing branch' '
-
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
 	test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
@@ -72,7 +70,6 @@ test_expect_success 'checkout from non-existing branch' '
 '
 
 test_expect_success 'checkout with dirty tree without -m' '
-
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
 	then
@@ -81,11 +78,9 @@ test_expect_success 'checkout with dirty tree without -m' '
 	else
 		echo "happy - failed correctly"
 	fi
-
 '
 
 test_expect_success 'checkout with unrelated dirty tree without -m' '
-
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
 	cp same kept &&
@@ -96,7 +91,6 @@ test_expect_success 'checkout with unrelated dirty tree without -m' '
 '
 
 test_expect_success 'checkout -m with dirty tree' '
-
 	git checkout -f master &&
 	git clean -f &&
 
@@ -121,7 +115,6 @@ test_expect_success 'checkout -m with dirty tree' '
 '
 
 test_expect_success 'checkout -m with dirty tree, renamed' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 2 3 4 5 7 8 >one &&
@@ -139,11 +132,9 @@ test_expect_success 'checkout -m with dirty tree, renamed' '
 	! test -f one &&
 	git diff --cached >current &&
 	test_must_be_empty current
-
 '
 
 test_expect_success 'checkout -m with merge conflict' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 T 3 4 5 6 S 8 >one &&
@@ -166,7 +157,6 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-
 	git checkout -f master && git clean -f &&
 
 	fill b d > two &&
@@ -190,7 +180,6 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 
 	fill b d > two &&
@@ -216,7 +205,6 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 	git rm two &&
 
@@ -228,7 +216,6 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 '
 
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
-
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
 	git checkout -f renamer && git clean -f &&
@@ -267,7 +254,6 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-
 	git checkout -f master && git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -283,7 +269,6 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-
 	git checkout -f master && git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -299,7 +284,6 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-
 	git checkout -f master && git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -315,7 +299,6 @@ test_expect_success 'checkout to detach HEAD with HEAD^0' '
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git tag both side &&
 	git branch both master &&
 	git reset --hard &&
@@ -327,11 +310,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	test "z$H" = "z$M" &&
 	name=$(git symbolic-ref HEAD 2>/dev/null) &&
 	test "z$name" = zrefs/heads/both
-
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -351,11 +332,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	else
 		: happy
 	fi
-
 '
 
 test_expect_success 'switch branches while in subdirectory' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -366,11 +345,9 @@ test_expect_success 'switch branches while in subdirectory' '
 	) &&
 	! test -f subs/one &&
 	rm -fr subs
-
 '
 
 test_expect_success 'checkout specific path while in subdirectory' '
-
 	git reset --hard &&
 	git checkout side &&
 	mkdir subs &&
@@ -385,7 +362,6 @@ test_expect_success 'checkout specific path while in subdirectory' '
 		git checkout side -- bero
 	) &&
 	test -f subs/bero
-
 '
 
 test_expect_success 'checkout w/--track sets up tracking' '
@@ -608,7 +584,6 @@ test_expect_success 'failing checkout -b should not break working tree' '
 	test $(git symbolic-ref HEAD) = refs/heads/master &&
 	git diff --exit-code &&
 	git diff --cached --exit-code
-
 '
 
 test_expect_success 'switch out of non-branch' '
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-20 11:43   ` [PATCH v3 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
@ 2020-10-20 11:43     ` Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 5/5][Outreachy] t7201: put each command on a seperate line Charvi Mendiratta
  4 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20 11:43 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, phillip.wood123, congdanhqx, me,
	Charvi Mendiratta

According to Documentation/CodingGuidelines, redirect
operator is written with space before, but no space
after them.

Let's remove these whitespaces after redirect operators.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7102-reset.sh | 30 +++++++++++++++---------------
 t/t7201-co.sh    | 10 +++++-----
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 2b4cfb2c83..a8c96bf162 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -70,15 +70,15 @@ check_changes () {
 
 test_expect_success 'reset --hard message' '
 	hex=$(git log -1 --format="%h") &&
-	git reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg) > .expected &&
+	git reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
 test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' '
 	hex=$(git log -1 --format="%h") &&
-	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg $test_encoding) > .expected &&
+	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg $test_encoding) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
@@ -387,25 +387,25 @@ test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge
 '
 
 test_expect_success 'test --mixed <paths>' '
-	echo 1 > file1 &&
-	echo 2 > file2 &&
+	echo 1 >file1 &&
+	echo 2 >file2 &&
 	git add file1 file2 &&
 	test_tick &&
 	git commit -m files &&
 	before1=$(git rev-parse --short HEAD:file1) &&
 	before2=$(git rev-parse --short HEAD:file2) &&
 	git rm file2 &&
-	echo 3 > file3 &&
-	echo 4 > file4 &&
-	echo 5 > file1 &&
+	echo 3 >file3 &&
+	echo 4 >file4 &&
+	echo 5 >file1 &&
 	after1=$(git rev-parse --short $(git hash-object file1)) &&
 	after4=$(git rev-parse --short $(git hash-object file4)) &&
 	git add file1 file3 file4 &&
 	git reset HEAD -- file1 file2 file3 &&
 	test_must_fail git diff --quiet &&
-	git diff > output &&
+	git diff >output &&
 
-	cat > expect <<-EOF &&
+	cat >expect <<-EOF &&
 	diff --git a/file1 b/file1
 	index $before1..$after1 100644
 	--- a/file1
@@ -423,9 +423,9 @@ test_expect_success 'test --mixed <paths>' '
 	EOF
 
 	test_cmp expect output &&
-	git diff --cached > output &&
+	git diff --cached >output &&
 
-	cat > cached_expect <<-EOF &&
+	cat >cached_expect <<-EOF &&
 	diff --git a/file4 b/file4
 	new file mode 100644
 	index 0000000..$after4
@@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
 	git diff-index --cached --exit-code HEAD
 '
 
-cat > expect << EOF
+cat >expect << EOF
 Unstaged changes after reset:
 M	file2
 EOF
 
 test_expect_success '--mixed refreshes the index' '
 	echo 123 >> file2 &&
-	git reset --mixed HEAD > output &&
+	git reset --mixed HEAD >output &&
 	test_i18ncmp expect output
 '
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index b527f8009c..74553f991b 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,7 @@ fill () {
 
 
 test_expect_success setup '
-	fill x y z > same &&
+	fill x y z >same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
 	git add same one two &&
@@ -55,7 +55,7 @@ test_expect_success setup '
 
 	git checkout -b simple master &&
 	rm -f one &&
-	fill a c e > two &&
+	fill a c e >two &&
 	git commit -a -m "Simple D one, M two" &&
 
 	git checkout master
@@ -95,7 +95,7 @@ test_expect_success 'checkout -m with dirty tree' '
 	git clean -f &&
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
-	git checkout -m side > messages &&
+	git checkout -m side >messages &&
 
 	test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
 
@@ -159,7 +159,7 @@ test_expect_success 'checkout -m with merge conflict' '
 test_expect_success 'format of merge conflict from checkout -m' '
 	git checkout -f master && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout -m simple &&
 
 	git ls-files >current &&
@@ -182,7 +182,7 @@ test_expect_success 'format of merge conflict from checkout -m' '
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 	git checkout -f master && git reset --hard && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
 
 	cat <<-EOF >expect &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 4/5][Outreachy] t7201: use 'git -C' to avoid subshell
  2020-10-20 11:43   ` [PATCH v3 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                       ` (2 preceding siblings ...)
  2020-10-20 11:43     ` [PATCH v3 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
@ 2020-10-20 11:43     ` Charvi Mendiratta
  2020-10-20 11:43     ` [PATCH v3 5/5][Outreachy] t7201: put each command on a seperate line Charvi Mendiratta
  4 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20 11:43 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, phillip.wood123, congdanhqx, me,
	Charvi Mendiratta

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 74553f991b..5898182fd2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
 	git checkout master &&
 
 	mkdir subs &&
-	(
-		cd subs &&
-		git checkout side
-	) &&
+	git -C subs checkout side &&
 	! test -f subs/one &&
 	rm -fr subs
 '
@@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 	git checkout master &&
 	mkdir -p subs &&
-	(
-		cd subs &&
-		git checkout side -- bero
-	) &&
+	git -C subs checkout side -- bero &&
 	test -f subs/bero
 '
 
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v3 5/5][Outreachy] t7201: put each command on a seperate line
  2020-10-20 11:43   ` [PATCH v3 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                       ` (3 preceding siblings ...)
  2020-10-20 11:43     ` [PATCH v3 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
@ 2020-10-20 11:43     ` Charvi Mendiratta
  4 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20 11:43 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, phillip.wood123, congdanhqx, me,
	Charvi Mendiratta

Modern practice is to avoid multiple commands per line,
and instead place each command on its own line.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5898182fd2..b36a93056f 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -157,7 +157,8 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout -m simple &&
@@ -180,7 +181,9 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
@@ -205,7 +208,9 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 	git rm two &&
 
 	test_must_fail git checkout simple 2>errs &&
@@ -218,7 +223,8 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	git checkout renamer^ 2>messages &&
 	test_i18ngrep "HEAD is now at $rev" messages &&
 	test_line_count = 1 messages &&
@@ -237,7 +243,8 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '
 test_expect_success 'checkout to detach HEAD' '
 	git config advice.detachedHead true &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
 	grep "HEAD is now at $rev" messages &&
 	test_line_count -gt 1 messages &&
@@ -254,7 +261,8 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -269,7 +277,8 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -284,7 +293,8 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-20  9:13               ` Phillip Wood
@ 2020-10-20 11:48                 ` Charvi Mendiratta
  0 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20 11:48 UTC (permalink / raw)
  To: phillip.wood
  Cc: Taylor Blau, git, Junio C Hamano, Christian Couder, Eric Sunshine

On Tue, 20 Oct 2020 at 14:43, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Charvi
>
> On 19/10/2020 21:25, Taylor Blau wrote:
> > Hi Charvi,
> >
> > On Mon, Oct 19, 2020 at 10:54:57PM +0530, Charvi Mendiratta wrote:
> >> Yes, thanks a lot Philip I understood the reason. I will do the corrections in
> >> commit message and commit body as below :
> >> t7201: using 'git -C' to avoid subshell
> >>
> >> Using 'git-C' instead of 'cd' inside of subshell, to avoid the extra process
> >> of starting a new subshell
> >>
> >> [...]
> >
> > That said, a couple of notes:
> >
> >    - Your subject message is good. It is concise, to-the-point, and
> >      accurately describes the change. Good.
> >
> >    - The body is similarly short, but could be rewritten to use the
> >      imperative mood. But, it is redundant with the subject. The subject
> >      says "we are using 'git -C' to avoid creating a subshell", and the
> >      patch says exactly the same.
> >
> > ...So, you can do one of two things. Either you can abbreviate the
> > subject, adding the additional detail in the patch message, or you could
> > leave the subject as-is and delete the patch message entirely.
> >
> > Either would be fine with me, but certainly Phillip or others could
> > chime in, too.
>
> I'm happy with either, but I would suggest changing the subject so the
> description starts with 'use' rather than 'using'
>

Thank you Phillip, I have updated the changes in the next patch .

> Best Wishes
>
> Phillip
>
> > Thanks,
> > Taylor
> >
Thanks and Regards,
Charvi

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v4] t7201: put each command on a separate line
  2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
                     ` (5 preceding siblings ...)
  2020-10-20 11:43   ` [PATCH v3 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
@ 2020-10-20 12:11   ` Charvi Mendiratta
  2020-10-20 20:13     ` Junio C Hamano
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
  6 siblings, 2 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-20 12:11 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, phillip.wood123, congdanhqx, me,
	Charvi Mendiratta

Modern practice is to avoid multiple commands per line,
and instead place each command on its own line.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5898182fd2..b36a93056f 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -157,7 +157,8 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout -m simple &&
@@ -180,7 +181,9 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
@@ -205,7 +208,9 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 	git rm two &&
 
 	test_must_fail git checkout simple 2>errs &&
@@ -218,7 +223,8 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	git checkout renamer^ 2>messages &&
 	test_i18ngrep "HEAD is now at $rev" messages &&
 	test_line_count = 1 messages &&
@@ -237,7 +243,8 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '
 test_expect_success 'checkout to detach HEAD' '
 	git config advice.detachedHead true &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
 	grep "HEAD is now at $rev" messages &&
 	test_line_count -gt 1 messages &&
@@ -254,7 +261,8 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -269,7 +277,8 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -284,7 +293,8 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells
  2020-10-20  5:38               ` Charvi Mendiratta
@ 2020-10-20 20:09                 ` Taylor Blau
  0 siblings, 0 replies; 60+ messages in thread
From: Taylor Blau @ 2020-10-20 20:09 UTC (permalink / raw)
  To: Charvi Mendiratta
  Cc: Taylor Blau, phillip.wood, git, Junio C Hamano, Christian Couder,
	Eric Sunshine

Hi Charvi,

On Tue, Oct 20, 2020 at 11:08:55AM +0530, Charvi Mendiratta wrote:
> > Usually it never hurts to just send the patch, since any feedback that a
> > reviewer has now is equally good even after you have sent a patch. Plus,
> > it's easier to review the concrete patch you want applied, instead of a
> > hypothetical of what you might send.
>
> Yes, I completely agree with you . Its my fault, I will send it in the
> patch and will
> take care of not repeating this again .

It's not your fault: there's no fault to be assigned when submitting
your first few patches to the list. You're doing much better than I did
;-).

> > That said, a couple of notes:
> >
> > [...]
> >
> Thanks Taylor, I will do the changes as you mentioned and send it in the
> next patch .

Thanks!
Taylor

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v4] t7201: put each command on a separate line
  2020-10-20 12:11   ` [PATCH v4] t7201: put each command on a separate line Charvi Mendiratta
@ 2020-10-20 20:13     ` Junio C Hamano
  2020-10-20 20:15       ` Taylor Blau
  2020-10-20 20:19       ` Junio C Hamano
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
  1 sibling, 2 replies; 60+ messages in thread
From: Junio C Hamano @ 2020-10-20 20:13 UTC (permalink / raw)
  To: Charvi Mendiratta; +Cc: git, christian.couder, phillip.wood123, congdanhqx, me

Charvi Mendiratta <charvi077@gmail.com> writes:

> Modern practice is to avoid multiple commands per line,
> and instead place each command on its own line.
>
> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> ---

This looks good, but I am wondering what happened between v3 and
v4.  

As you've demonstrated through the microproject that you can now
comfortably be involved in the review discussion, I am tempted to
suggest that we declare victory at this point and move on, but I
don't know what the plans are for the other 4 patches (I guess we
won't miss them that much---the micros are meant to be practice
targets).

Thanks.


>  t/t7201-co.sh | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 5898182fd2..b36a93056f 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -157,7 +157,8 @@ test_expect_success 'checkout -m with merge conflict' '
>  '
>  
>  test_expect_success 'format of merge conflict from checkout -m' '
> -	git checkout -f master && git clean -f &&
> +	git checkout -f master &&
> +	git clean -f &&
>  
>  	fill b d >two &&
>  	git checkout -m simple &&
> @@ -180,7 +181,9 @@ test_expect_success 'format of merge conflict from checkout -m' '
>  '
>  
>  test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
> -	git checkout -f master && git reset --hard && git clean -f &&
> +	git checkout -f master &&
> +	git reset --hard &&
> +	git clean -f &&
>  
>  	fill b d >two &&
>  	git checkout --merge --conflict=diff3 simple &&
> @@ -205,7 +208,9 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
>  '
>  
>  test_expect_success 'switch to another branch while carrying a deletion' '
> -	git checkout -f master && git reset --hard && git clean -f &&
> +	git checkout -f master &&
> +	git reset --hard &&
> +	git clean -f &&
>  	git rm two &&
>  
>  	test_must_fail git checkout simple 2>errs &&
> @@ -218,7 +223,8 @@ test_expect_success 'switch to another branch while carrying a deletion' '
>  test_expect_success 'checkout to detach HEAD (with advice declined)' '
>  	git config advice.detachedHead false &&
>  	rev=$(git rev-parse --short renamer^) &&
> -	git checkout -f renamer && git clean -f &&
> +	git checkout -f renamer &&
> +	git clean -f &&
>  	git checkout renamer^ 2>messages &&
>  	test_i18ngrep "HEAD is now at $rev" messages &&
>  	test_line_count = 1 messages &&
> @@ -237,7 +243,8 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '
>  test_expect_success 'checkout to detach HEAD' '
>  	git config advice.detachedHead true &&
>  	rev=$(git rev-parse --short renamer^) &&
> -	git checkout -f renamer && git clean -f &&
> +	git checkout -f renamer &&
> +	git clean -f &&
>  	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
>  	grep "HEAD is now at $rev" messages &&
>  	test_line_count -gt 1 messages &&
> @@ -254,7 +261,8 @@ test_expect_success 'checkout to detach HEAD' '
>  '
>  
>  test_expect_success 'checkout to detach HEAD with branchname^' '
> -	git checkout -f master && git clean -f &&
> +	git checkout -f master &&
> +	git clean -f &&
>  	git checkout renamer^ &&
>  	H=$(git rev-parse --verify HEAD) &&
>  	M=$(git show-ref -s --verify refs/heads/master) &&
> @@ -269,7 +277,8 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
>  '
>  
>  test_expect_success 'checkout to detach HEAD with :/message' '
> -	git checkout -f master && git clean -f &&
> +	git checkout -f master &&
> +	git clean -f &&
>  	git checkout ":/Initial" &&
>  	H=$(git rev-parse --verify HEAD) &&
>  	M=$(git show-ref -s --verify refs/heads/master) &&
> @@ -284,7 +293,8 @@ test_expect_success 'checkout to detach HEAD with :/message' '
>  '
>  
>  test_expect_success 'checkout to detach HEAD with HEAD^0' '
> -	git checkout -f master && git clean -f &&
> +	git checkout -f master &&
> +	git clean -f &&
>  	git checkout HEAD^0 &&
>  	H=$(git rev-parse --verify HEAD) &&
>  	M=$(git show-ref -s --verify refs/heads/master) &&

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v4] t7201: put each command on a separate line
  2020-10-20 20:13     ` Junio C Hamano
@ 2020-10-20 20:15       ` Taylor Blau
  2020-10-20 20:25         ` Junio C Hamano
  2020-10-20 20:19       ` Junio C Hamano
  1 sibling, 1 reply; 60+ messages in thread
From: Taylor Blau @ 2020-10-20 20:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Charvi Mendiratta, git, christian.couder, phillip.wood123,
	congdanhqx, me

On Tue, Oct 20, 2020 at 01:13:53PM -0700, Junio C Hamano wrote:
> Charvi Mendiratta <charvi077@gmail.com> writes:
>
> > Modern practice is to avoid multiple commands per line,
> > and instead place each command on its own line.
> >
> > Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> > ---
>
> This looks good, but I am wondering what happened between v3 and
> v4.

When I applied this locally, I used this patch as a replacement for the
last patch of v3 [1]. That kept everything passing after each patch.

> As you've demonstrated through the microproject that you can now
> comfortably be involved in the review discussion, I am tempted to
> suggest that we declare victory at this point and move on, but I
> don't know what the plans are for the other 4 patches (I guess we
> won't miss them that much---the micros are meant to be practice
> targets).

Yup, ditto.

> Thanks.

Thanks,
Taylor

[1]: https://lore.kernel.org/git/20201020114319.18245-6-charvi077@gmail.com/

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v4] t7201: put each command on a separate line
  2020-10-20 20:13     ` Junio C Hamano
  2020-10-20 20:15       ` Taylor Blau
@ 2020-10-20 20:19       ` Junio C Hamano
  2020-10-21 13:16         ` Charvi Mendiratta
  1 sibling, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2020-10-20 20:19 UTC (permalink / raw)
  To: Charvi Mendiratta; +Cc: git, christian.couder, phillip.wood123, congdanhqx, me

Junio C Hamano <gitster@pobox.com> writes:

> Charvi Mendiratta <charvi077@gmail.com> writes:
>
>> Modern practice is to avoid multiple commands per line,
>> and instead place each command on its own line.
>>
>> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
>> ---
>
> This looks good, but I am wondering what happened between v3 and
> v4.  
>
> As you've demonstrated through the microproject that you can now
> comfortably be involved in the review discussion, I am tempted to
> suggest that we declare victory at this point and move on, but I
> don't know what the plans are for the other 4 patches (I guess we
> won't miss them that much---the micros are meant to be practice
> targets).

Actually I take it back.  This does not look good as a standalone
patch at all.  It seems to depend on something in the 5-patch
series.

Please make sure that patches you send are usable by your
recipients.


^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v4] t7201: put each command on a separate line
  2020-10-20 20:15       ` Taylor Blau
@ 2020-10-20 20:25         ` Junio C Hamano
  2020-10-20 20:30           ` Taylor Blau
  0 siblings, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2020-10-20 20:25 UTC (permalink / raw)
  To: Taylor Blau
  Cc: Charvi Mendiratta, git, christian.couder, phillip.wood123, congdanhqx

Taylor Blau <me@ttaylorr.com> writes:

> On Tue, Oct 20, 2020 at 01:13:53PM -0700, Junio C Hamano wrote:
>> Charvi Mendiratta <charvi077@gmail.com> writes:
>>
>> > Modern practice is to avoid multiple commands per line,
>> > and instead place each command on its own line.
>> >
>> > Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
>> > ---
>>
>> This looks good, but I am wondering what happened between v3 and
>> v4.
>
> When I applied this locally, I used this patch as a replacement for the
> last patch of v3 [1]. That kept everything passing after each patch.

Oh, so this is a replacement for 5/5 and 1-4/5 of v4 are supposed to
be identical to those from v3?  The difference between [v3 5/5] and
this one is a single typofix on the subject line, it seems, though.

>> As you've demonstrated through the microproject that you can now
>> comfortably be involved in the review discussion, I am tempted to
>> suggest that we declare victory at this point and move on, but I
>> don't know what the plans are for the other 4 patches (I guess we
>> won't miss them that much---the micros are meant to be practice
>> targets).
>
> Yup, ditto.

As [v4] single patch won't apply standalone, we cannot quite declare
the victory yet.  Are [v3 1-5/5] (or [v3 1-4/5] + [v4]) good to the
reviewers of the past rounds?

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v4] t7201: put each command on a separate line
  2020-10-20 20:25         ` Junio C Hamano
@ 2020-10-20 20:30           ` Taylor Blau
  2020-10-20 21:00             ` Junio C Hamano
  2020-10-21  7:14             ` Charvi Mendiratta
  0 siblings, 2 replies; 60+ messages in thread
From: Taylor Blau @ 2020-10-20 20:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Taylor Blau, Charvi Mendiratta, git, christian.couder,
	phillip.wood123, congdanhqx

On Tue, Oct 20, 2020 at 01:25:33PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
> > When I applied this locally, I used this patch as a replacement for the
> > last patch of v3 [1]. That kept everything passing after each patch.
>
> Oh, so this is a replacement for 5/5 and 1-4/5 of v4 are supposed to
> be identical to those from v3?  The difference between [v3 5/5] and
> this one is a single typofix on the subject line, it seems, though.

Yes, at least that's what I interpreted it as (and how I applied it when
testing). I'd like to hear from the author to make sure.

(As an aside to the author, I often fall into the trap of thinking that
it will be easier to send a single replacement patch which will generate
less email, but--as you can see--it is often more complicated for
reviewers and the maintainer to decipher what's going on. It's often
just easier to re-submit the entire series and include in your cover
letter "this is unchanged from v(n-1) except for ...").

> >> As you've demonstrated through the microproject that you can now
> >> comfortably be involved in the review discussion, I am tempted to
> >> suggest that we declare victory at this point and move on, but I
> >> don't know what the plans are for the other 4 patches (I guess we
> >> won't miss them that much---the micros are meant to be practice
> >> targets).
> >
> > Yup, ditto.
>
> As [v4] single patch won't apply standalone, we cannot quite declare
> the victory yet.  Are [v3 1-5/5] (or [v3 1-4/5] + [v4]) good to the
> reviewers of the past rounds?

For what it's worth, I'm happy with [v3 1-4/5] + [v4].

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v4] t7201: put each command on a separate line
  2020-10-20 20:30           ` Taylor Blau
@ 2020-10-20 21:00             ` Junio C Hamano
  2020-10-21  7:14             ` Charvi Mendiratta
  1 sibling, 0 replies; 60+ messages in thread
From: Junio C Hamano @ 2020-10-20 21:00 UTC (permalink / raw)
  To: Taylor Blau
  Cc: Charvi Mendiratta, git, christian.couder, phillip.wood123, congdanhqx

Taylor Blau <me@ttaylorr.com> writes:

>> As [v4] single patch won't apply standalone, we cannot quite declare
>> the victory yet.  Are [v3 1-5/5] (or [v3 1-4/5] + [v4]) good to the
>> reviewers of the past rounds?
>
> For what it's worth, I'm happy with [v3 1-4/5] + [v4].

Yeah, I'm happy with them, but this is an impression only from a
quick skimming---I guess I'd just trust you and won't go back to the
patches with fine toothed comb myself ;-).

Thanks, all.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v4] t7201: put each command on a separate line
  2020-10-20 20:30           ` Taylor Blau
  2020-10-20 21:00             ` Junio C Hamano
@ 2020-10-21  7:14             ` Charvi Mendiratta
  1 sibling, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-21  7:14 UTC (permalink / raw)
  To: Taylor Blau
  Cc: Junio C Hamano, git, Christian Couder, phillip.wood123,
	Đoàn Trần Công Danh

On Wed, 21 Oct 2020 at 02:00, Taylor Blau <me@ttaylorr.com> wrote:
>
> On Tue, Oct 20, 2020 at 01:25:33PM -0700, Junio C Hamano wrote:
> > Taylor Blau <me@ttaylorr.com> writes:
> > > When I applied this locally, I used this patch as a replacement for the
> > > last patch of v3 [1]. That kept everything passing after each patch.
> >
> > Oh, so this is a replacement for 5/5 and 1-4/5 of v4 are supposed to
> > be identical to those from v3?  The difference between [v3 5/5] and
> > this one is a single typofix on the subject line, it seems, though.
>
> Yes, at least that's what I interpreted it as (and how I applied it when
> testing). I'd like to hear from the author to make sure.
>

I think I messed up the versions. Its correct that v4 patch was only
replacement for 5/5 (5th patch) of v3, since I need  to fix the typo
error of subject line. Also, other 4 patches (1-4/5) of v3 need to be
remain same in v4.

> (As an aside to the author, I often fall into the trap of thinking that
> it will be easier to send a single replacement patch which will generate
> less email, but--as you can see--it is often more complicated for
> reviewers and the maintainer to decipher what's going on. It's often
> just easier to re-submit the entire series and include in your cover
> letter "this is unchanged from v(n-1) except for ...").
>

Yes I realized this, actually earlier I was doubtful about whether to include
the previous version's correct patches in the new version or not. I might
have confirmed this before sending. But now I will strictly follow this .

Thanks a lot to Junio and Taylor for pointing this out. And in order to
correct this, I will send the new patch series having (v3 1-4/5]+[v4]).

Please correct me, if I missed out anything else.

> > >> As you've demonstrated through the microproject that you can now
> > >> comfortably be involved in the review discussion, I am tempted to
> > >> suggest that we declare victory at this point and move on, but I
> > >> don't know what the plans are for the other 4 patches (I guess we
> > >> won't miss them that much---the micros are meant to be practice
> > >> targets).
> > >
> > > Yup, ditto.
> >
> > As [v4] single patch won't apply standalone, we cannot quite declare
> > the victory yet.  Are [v3 1-5/5] (or [v3 1-4/5] + [v4]) good to the
> > reviewers of the past rounds?
>
> For what it's worth, I'm happy with [v3 1-4/5] + [v4].
>
> Thanks,
> Taylor

Thanks and Regards,
Charvi

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v5 0/5][Outreachy] modernize the test scripts
  2020-10-20 12:11   ` [PATCH v4] t7201: put each command on a separate line Charvi Mendiratta
  2020-10-20 20:13     ` Junio C Hamano
@ 2020-10-21 12:48     ` Charvi Mendiratta
  2020-10-21 12:48       ` [PATCH v5 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
                         ` (10 more replies)
  1 sibling, 11 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-21 12:48 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, me, phillip.wood123, Charvi Mendiratta

In this patch series, I have combined all the correct patches
from version 3 and version 4 and this patch series consists of
(v3 1-4/5 + v4) patches, else everything remains unchanged.
I have also tested by applying it locally, in order to confirm 
that these patches are usable.

Thanks to Junio and Taylor for the guidance.

This patch series :
 -modernize the three test scripts : t7101, t7201 and t7102.
 - cleans up with 5 types of changes in all the three scripts.
   1. Converting the old style test format to new one
   2. Removing blankspaces in test bodies
   3. Removing whitespaces after the redirect operator, according to
      CodingGuidelines 
   4. Using 'git -C' to avoid use of another subshell 
   5. Placing commands in separate lines


Charvi Mendiratta (5):
  t7101,t7102,t7201: modernize test formatting
  t7102,t7201: remove unnecessary blank spaces in test body
  t7102,t7201: remove whitespace after redirect operator
  t7201: use 'git -C' to avoid subshell
  t7201: put each command on a separate line

 t/t7101-reset-empty-subdirs.sh |  66 ++++++++++-----------
 t/t7102-reset.sh               |  63 ++++++++------------
 t/t7201-co.sh                  | 102 +++++++++++++--------------------
 3 files changed, 96 insertions(+), 135 deletions(-)

-- 
2.29.0.rc1


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v5 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
@ 2020-10-21 12:48       ` Charvi Mendiratta
  2020-10-21 12:48       ` [PATCH v5 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
                         ` (9 subsequent siblings)
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-21 12:48 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, me, phillip.wood123, Charvi Mendiratta

Some tests in these scripts are formatted using a very old style:
        test_expect_success \
            'title' \
            'body line 1 &&
             body line 2'

Updating the formatting to the modern style:
        test_expect_success 'title' '
            body line 1 &&
            body line 2
        '

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7101-reset-empty-subdirs.sh | 66 +++++++++++++++++-----------------
 t/t7102-reset.sh               | 24 +++++--------
 t/t7201-co.sh                  | 31 ++++++++--------
 3 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/t/t7101-reset-empty-subdirs.sh b/t/t7101-reset-empty-subdirs.sh
index 96e163f084..bfce05ac5d 100755
--- a/t/t7101-reset-empty-subdirs.sh
+++ b/t/t7101-reset-empty-subdirs.sh
@@ -6,16 +6,15 @@
 test_description='git reset should cull empty subdirs'
 . ./test-lib.sh
 
-test_expect_success \
-    'creating initial files' \
-    'mkdir path0 &&
+test_expect_success 'creating initial files' '
+     mkdir path0 &&
      cp "$TEST_DIRECTORY"/../COPYING path0/COPYING &&
      git add path0/COPYING &&
-     git commit -m add -a'
+     git commit -m add -a
+'
 
-test_expect_success \
-    'creating second files' \
-    'mkdir path1 &&
+test_expect_success 'creating second files' '
+     mkdir path1 &&
      mkdir path1/path2 &&
      cp "$TEST_DIRECTORY"/../COPYING path1/path2/COPYING &&
      cp "$TEST_DIRECTORY"/../COPYING path1/COPYING &&
@@ -25,39 +24,40 @@ test_expect_success \
      git add path1/COPYING &&
      git add COPYING &&
      git add path0/COPYING-TOO &&
-     git commit -m change -a'
+     git commit -m change -a
+'
 
-test_expect_success \
-    'resetting tree HEAD^' \
-    'git reset --hard HEAD^'
+test_expect_success 'resetting tree HEAD^' '
+     git reset --hard HEAD^
+'
 
-test_expect_success \
-    'checking initial files exist after rewind' \
-    'test -d path0 &&
-     test -f path0/COPYING'
+test_expect_success 'checking initial files exist after rewind' '
+     test -d path0 &&
+     test -f path0/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/path2/COPYING' \
-    '! test -f path1/path2/COPYING'
+test_expect_success 'checking lack of path1/path2/COPYING' '
+    ! test -f path1/path2/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/COPYING' \
-    '! test -f path1/COPYING'
+test_expect_success 'checking lack of path1/COPYING' '
+    ! test -f path1/COPYING
+'
 
-test_expect_success \
-    'checking lack of COPYING' \
-    '! test -f COPYING'
+test_expect_success 'checking lack of COPYING' '
+     ! test -f COPYING
+'
 
-test_expect_success \
-    'checking checking lack of path1/COPYING-TOO' \
-    '! test -f path0/COPYING-TOO'
+test_expect_success 'checking checking lack of path1/COPYING-TOO' '
+     ! test -f path0/COPYING-TOO
+'
 
-test_expect_success \
-    'checking lack of path1/path2' \
-    '! test -d path1/path2'
+test_expect_success 'checking lack of path1/path2' '
+     ! test -d path1/path2
+'
 
-test_expect_success \
-    'checking lack of path1' \
-    '! test -d path1'
+test_expect_success 'checking lack of path1' '
+     ! test -d path1
+'
 
 test_done
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 22161b3b2d..fe43f77513 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -107,8 +107,7 @@ test_expect_success 'reset --soft with unmerged index should fail' '
 	git rm --cached -- un
 '
 
-test_expect_success \
-	'giving paths with options different than --mixed should fail' '
+test_expect_success 'giving paths with options different than --mixed should fail' '
 	test_must_fail git reset --soft -- first &&
 	test_must_fail git reset --hard -- first &&
 	test_must_fail git reset --soft HEAD^ -- first &&
@@ -128,8 +127,7 @@ test_expect_success 'giving unrecognized options should fail' '
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending merge should fail' '
+test_expect_success 'trying to do reset --soft with pending merge should fail' '
 	git branch branch1 &&
 	git branch branch2 &&
 
@@ -152,8 +150,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending checkout merge should fail' '
+test_expect_success 'trying to do reset --soft with pending checkout merge should fail' '
 	git branch branch3 &&
 	git branch branch4 &&
 
@@ -175,8 +172,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'resetting to HEAD with no changes should succeed and do nothing' '
+test_expect_success 'resetting to HEAD with no changes should succeed and do nothing' '
 	git reset --hard &&
 		check_changes $head5 &&
 	git reset --hard HEAD &&
@@ -226,8 +222,7 @@ secondfile:
 2nd line 2nd file
 3rd line 2nd file
 EOF
-test_expect_success \
-	'changing files and redo the last commit should succeed' '
+test_expect_success 'changing files and redo the last commit should succeed' '
 	echo "3rd line 2nd file" >>secondfile &&
 	git commit -a -C ORIG_HEAD &&
 	head4=$(git rev-parse --verify HEAD) &&
@@ -245,8 +240,7 @@ first:
 second:
 2nd file
 EOF
-test_expect_success \
-	'--hard reset should change the files and undo commits permanently' '
+test_expect_success '--hard reset should change the files and undo commits permanently' '
 	git reset --hard HEAD~2 &&
 	check_changes $head5p2 &&
 	test "$(git rev-parse ORIG_HEAD)" = \
@@ -284,8 +278,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'redoing changes adding them without commit them should succeed' '
+test_expect_success 'redoing changes adding them without commit them should succeed' '
 	git rm first &&
 	git mv second secondfile &&
 
@@ -380,8 +373,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'--hard reset to ORIG_HEAD should clear a fast-forward merge' '
+test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge' '
 	git reset --hard HEAD^ &&
 	check_changes $head5 &&
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 4d62b9b00f..a800bda5e3 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -62,7 +62,7 @@ test_expect_success setup '
 	git checkout master
 '
 
-test_expect_success "checkout from non-existing branch" '
+test_expect_success 'checkout from non-existing branch' '
 
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
@@ -71,7 +71,7 @@ test_expect_success "checkout from non-existing branch" '
 	test refs/heads/master = "$(git symbolic-ref HEAD)"
 '
 
-test_expect_success "checkout with dirty tree without -m" '
+test_expect_success 'checkout with dirty tree without -m' '
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
@@ -84,7 +84,7 @@ test_expect_success "checkout with dirty tree without -m" '
 
 '
 
-test_expect_success "checkout with unrelated dirty tree without -m" '
+test_expect_success 'checkout with unrelated dirty tree without -m' '
 
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
@@ -95,7 +95,7 @@ test_expect_success "checkout with unrelated dirty tree without -m" '
 	test_cmp messages.expect messages
 '
 
-test_expect_success "checkout -m with dirty tree" '
+test_expect_success 'checkout -m with dirty tree' '
 
 	git checkout -f master &&
 	git clean -f &&
@@ -120,7 +120,7 @@ test_expect_success "checkout -m with dirty tree" '
 	test_must_be_empty current.index
 '
 
-test_expect_success "checkout -m with dirty tree, renamed" '
+test_expect_success 'checkout -m with dirty tree, renamed' '
 
 	git checkout -f master && git clean -f &&
 
@@ -388,22 +388,22 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 '
 
-test_expect_success \
-    'checkout w/--track sets up tracking' '
+test_expect_success 'checkout w/--track sets up tracking' '
     git config branch.autosetupmerge false &&
     git checkout master &&
     git checkout --track -b track1 &&
     test "$(git config branch.track1.remote)" &&
-    test "$(git config branch.track1.merge)"'
+    test "$(git config branch.track1.merge)"
+'
 
-test_expect_success \
-    'checkout w/autosetupmerge=always sets up tracking' '
+test_expect_success 'checkout w/autosetupmerge=always sets up tracking' '
     test_when_finished git config branch.autosetupmerge false &&
     git config branch.autosetupmerge always &&
     git checkout master &&
     git checkout -b track2 &&
     test "$(git config branch.track2.remote)" &&
-    test "$(git config branch.track2.merge)"'
+    test "$(git config branch.track2.merge)"
+'
 
 test_expect_success 'checkout w/--track from non-branch HEAD fails' '
     git checkout master^0 &&
@@ -435,8 +435,7 @@ test_expect_success 'detach a symbolic link HEAD' '
     test "z$(git rev-parse --verify refs/heads/master)" = "z$here"
 '
 
-test_expect_success \
-    'checkout with --track fakes a sensible -b <name>' '
+test_expect_success 'checkout with --track fakes a sensible -b <name>' '
     git config remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" &&
     git update-ref refs/remotes/origin/koala/bear renamer &&
 
@@ -457,9 +456,9 @@ test_expect_success \
     test "$(git rev-parse HEAD)" = "$(git rev-parse renamer)"
 '
 
-test_expect_success \
-    'checkout with --track, but without -b, fails with too short tracked name' '
-    test_must_fail git checkout --track renamer'
+test_expect_success 'checkout with --track, but without -b, fails with too short tracked name' '
+    test_must_fail git checkout --track renamer
+'
 
 setup_conflicting_index () {
 	rm -f .git/index &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v5 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
  2020-10-21 12:48       ` [PATCH v5 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
@ 2020-10-21 12:48       ` Charvi Mendiratta
  2020-10-21 12:48       ` [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
                         ` (8 subsequent siblings)
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-21 12:48 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, me, phillip.wood123, Charvi Mendiratta

t7102 and t7201 still follow the old style of having blank
lines around test body, which is not consistence with our
current practice.

Let's remove those unnecessary blank lines.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7102-reset.sh |  9 ---------
 t/t7201-co.sh    | 25 -------------------------
 2 files changed, 34 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index fe43f77513..2b4cfb2c83 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -439,7 +439,6 @@ test_expect_success 'test --mixed <paths>' '
 '
 
 test_expect_success 'test resetting the index at give paths' '
-
 	mkdir sub &&
 	>sub/file1 &&
 	>sub/file2 &&
@@ -452,7 +451,6 @@ test_expect_success 'test resetting the index at give paths' '
 	echo "$U" &&
 	test_must_fail git diff-index --cached --exit-code "$T" &&
 	test "$T" != "$U"
-
 '
 
 test_expect_success 'resetting an unmodified path is a no-op' '
@@ -490,7 +488,6 @@ test_expect_success 'resetting specific path that is unmerged' '
 '
 
 test_expect_success 'disambiguation (1)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -499,11 +496,9 @@ test_expect_success 'disambiguation (1)' '
 	test -z "$(git diff --cached --name-only)" &&
 	test -f secondfile &&
 	test_must_be_empty secondfile
-
 '
 
 test_expect_success 'disambiguation (2)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -511,11 +506,9 @@ test_expect_success 'disambiguation (2)' '
 	test_must_fail git reset secondfile &&
 	test -n "$(git diff --cached --name-only -- secondfile)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (3)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -524,11 +517,9 @@ test_expect_success 'disambiguation (3)' '
 	test_must_fail git diff --quiet &&
 	test -z "$(git diff --cached --name-only)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (4)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index a800bda5e3..b527f8009c 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,6 @@ fill () {
 
 
 test_expect_success setup '
-
 	fill x y z > same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
@@ -63,7 +62,6 @@ test_expect_success setup '
 '
 
 test_expect_success 'checkout from non-existing branch' '
-
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
 	test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
@@ -72,7 +70,6 @@ test_expect_success 'checkout from non-existing branch' '
 '
 
 test_expect_success 'checkout with dirty tree without -m' '
-
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
 	then
@@ -81,11 +78,9 @@ test_expect_success 'checkout with dirty tree without -m' '
 	else
 		echo "happy - failed correctly"
 	fi
-
 '
 
 test_expect_success 'checkout with unrelated dirty tree without -m' '
-
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
 	cp same kept &&
@@ -96,7 +91,6 @@ test_expect_success 'checkout with unrelated dirty tree without -m' '
 '
 
 test_expect_success 'checkout -m with dirty tree' '
-
 	git checkout -f master &&
 	git clean -f &&
 
@@ -121,7 +115,6 @@ test_expect_success 'checkout -m with dirty tree' '
 '
 
 test_expect_success 'checkout -m with dirty tree, renamed' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 2 3 4 5 7 8 >one &&
@@ -139,11 +132,9 @@ test_expect_success 'checkout -m with dirty tree, renamed' '
 	! test -f one &&
 	git diff --cached >current &&
 	test_must_be_empty current
-
 '
 
 test_expect_success 'checkout -m with merge conflict' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 T 3 4 5 6 S 8 >one &&
@@ -166,7 +157,6 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-
 	git checkout -f master && git clean -f &&
 
 	fill b d > two &&
@@ -190,7 +180,6 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 
 	fill b d > two &&
@@ -216,7 +205,6 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 	git rm two &&
 
@@ -228,7 +216,6 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 '
 
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
-
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
 	git checkout -f renamer && git clean -f &&
@@ -267,7 +254,6 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-
 	git checkout -f master && git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -283,7 +269,6 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-
 	git checkout -f master && git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -299,7 +284,6 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-
 	git checkout -f master && git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -315,7 +299,6 @@ test_expect_success 'checkout to detach HEAD with HEAD^0' '
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git tag both side &&
 	git branch both master &&
 	git reset --hard &&
@@ -327,11 +310,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	test "z$H" = "z$M" &&
 	name=$(git symbolic-ref HEAD 2>/dev/null) &&
 	test "z$name" = zrefs/heads/both
-
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -351,11 +332,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	else
 		: happy
 	fi
-
 '
 
 test_expect_success 'switch branches while in subdirectory' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -366,11 +345,9 @@ test_expect_success 'switch branches while in subdirectory' '
 	) &&
 	! test -f subs/one &&
 	rm -fr subs
-
 '
 
 test_expect_success 'checkout specific path while in subdirectory' '
-
 	git reset --hard &&
 	git checkout side &&
 	mkdir subs &&
@@ -385,7 +362,6 @@ test_expect_success 'checkout specific path while in subdirectory' '
 		git checkout side -- bero
 	) &&
 	test -f subs/bero
-
 '
 
 test_expect_success 'checkout w/--track sets up tracking' '
@@ -608,7 +584,6 @@ test_expect_success 'failing checkout -b should not break working tree' '
 	test $(git symbolic-ref HEAD) = refs/heads/master &&
 	git diff --exit-code &&
 	git diff --cached --exit-code
-
 '
 
 test_expect_success 'switch out of non-branch' '
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
  2020-10-21 12:48       ` [PATCH v5 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
  2020-10-21 12:48       ` [PATCH v5 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
@ 2020-10-21 12:48       ` Charvi Mendiratta
  2020-10-21 17:20         ` Eric Sunshine
  2020-10-21 12:48       ` [PATCH v5 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
                         ` (7 subsequent siblings)
  10 siblings, 1 reply; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-21 12:48 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, me, phillip.wood123, Charvi Mendiratta

According to Documentation/CodingGuidelines, redirect
operator is written with space before, but no space
after them.

Let's remove these whitespaces after redirect operators.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7102-reset.sh | 30 +++++++++++++++---------------
 t/t7201-co.sh    | 10 +++++-----
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 2b4cfb2c83..a8c96bf162 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -70,15 +70,15 @@ check_changes () {
 
 test_expect_success 'reset --hard message' '
 	hex=$(git log -1 --format="%h") &&
-	git reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg) > .expected &&
+	git reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
 test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' '
 	hex=$(git log -1 --format="%h") &&
-	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg $test_encoding) > .expected &&
+	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg $test_encoding) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
@@ -387,25 +387,25 @@ test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge
 '
 
 test_expect_success 'test --mixed <paths>' '
-	echo 1 > file1 &&
-	echo 2 > file2 &&
+	echo 1 >file1 &&
+	echo 2 >file2 &&
 	git add file1 file2 &&
 	test_tick &&
 	git commit -m files &&
 	before1=$(git rev-parse --short HEAD:file1) &&
 	before2=$(git rev-parse --short HEAD:file2) &&
 	git rm file2 &&
-	echo 3 > file3 &&
-	echo 4 > file4 &&
-	echo 5 > file1 &&
+	echo 3 >file3 &&
+	echo 4 >file4 &&
+	echo 5 >file1 &&
 	after1=$(git rev-parse --short $(git hash-object file1)) &&
 	after4=$(git rev-parse --short $(git hash-object file4)) &&
 	git add file1 file3 file4 &&
 	git reset HEAD -- file1 file2 file3 &&
 	test_must_fail git diff --quiet &&
-	git diff > output &&
+	git diff >output &&
 
-	cat > expect <<-EOF &&
+	cat >expect <<-EOF &&
 	diff --git a/file1 b/file1
 	index $before1..$after1 100644
 	--- a/file1
@@ -423,9 +423,9 @@ test_expect_success 'test --mixed <paths>' '
 	EOF
 
 	test_cmp expect output &&
-	git diff --cached > output &&
+	git diff --cached >output &&
 
-	cat > cached_expect <<-EOF &&
+	cat >cached_expect <<-EOF &&
 	diff --git a/file4 b/file4
 	new file mode 100644
 	index 0000000..$after4
@@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
 	git diff-index --cached --exit-code HEAD
 '
 
-cat > expect << EOF
+cat >expect << EOF
 Unstaged changes after reset:
 M	file2
 EOF
 
 test_expect_success '--mixed refreshes the index' '
 	echo 123 >> file2 &&
-	git reset --mixed HEAD > output &&
+	git reset --mixed HEAD >output &&
 	test_i18ncmp expect output
 '
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index b527f8009c..74553f991b 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,7 @@ fill () {
 
 
 test_expect_success setup '
-	fill x y z > same &&
+	fill x y z >same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
 	git add same one two &&
@@ -55,7 +55,7 @@ test_expect_success setup '
 
 	git checkout -b simple master &&
 	rm -f one &&
-	fill a c e > two &&
+	fill a c e >two &&
 	git commit -a -m "Simple D one, M two" &&
 
 	git checkout master
@@ -95,7 +95,7 @@ test_expect_success 'checkout -m with dirty tree' '
 	git clean -f &&
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
-	git checkout -m side > messages &&
+	git checkout -m side >messages &&
 
 	test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
 
@@ -159,7 +159,7 @@ test_expect_success 'checkout -m with merge conflict' '
 test_expect_success 'format of merge conflict from checkout -m' '
 	git checkout -f master && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout -m simple &&
 
 	git ls-files >current &&
@@ -182,7 +182,7 @@ test_expect_success 'format of merge conflict from checkout -m' '
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 	git checkout -f master && git reset --hard && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
 
 	cat <<-EOF >expect &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v5 4/5][Outreachy] t7201: use 'git -C' to avoid subshell
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                         ` (2 preceding siblings ...)
  2020-10-21 12:48       ` [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
@ 2020-10-21 12:48       ` Charvi Mendiratta
  2020-10-21 12:48       ` [PATCH v5 5/5][Outreachy] t7201: put each command on a separate line Charvi Mendiratta
                         ` (6 subsequent siblings)
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-21 12:48 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, me, phillip.wood123, Charvi Mendiratta

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 74553f991b..5898182fd2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
 	git checkout master &&
 
 	mkdir subs &&
-	(
-		cd subs &&
-		git checkout side
-	) &&
+	git -C subs checkout side &&
 	! test -f subs/one &&
 	rm -fr subs
 '
@@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 	git checkout master &&
 	mkdir -p subs &&
-	(
-		cd subs &&
-		git checkout side -- bero
-	) &&
+	git -C subs checkout side -- bero &&
 	test -f subs/bero
 '
 
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v5 5/5][Outreachy] t7201: put each command on a separate line
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                         ` (3 preceding siblings ...)
  2020-10-21 12:48       ` [PATCH v5 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
@ 2020-10-21 12:48       ` Charvi Mendiratta
  2020-10-22  7:16       ` [PATCH v6 0/5][Outreachy] modernize test scripts Charvi Mendiratta
                         ` (5 subsequent siblings)
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-21 12:48 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, me, phillip.wood123, Charvi Mendiratta

Modern practice is to avoid multiple commands per line,
and instead place each command on its own line.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5898182fd2..b36a93056f 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -157,7 +157,8 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout -m simple &&
@@ -180,7 +181,9 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
@@ -205,7 +208,9 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 	git rm two &&
 
 	test_must_fail git checkout simple 2>errs &&
@@ -218,7 +223,8 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	git checkout renamer^ 2>messages &&
 	test_i18ngrep "HEAD is now at $rev" messages &&
 	test_line_count = 1 messages &&
@@ -237,7 +243,8 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '
 test_expect_success 'checkout to detach HEAD' '
 	git config advice.detachedHead true &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
 	grep "HEAD is now at $rev" messages &&
 	test_line_count -gt 1 messages &&
@@ -254,7 +261,8 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -269,7 +277,8 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -284,7 +293,8 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH v4] t7201: put each command on a separate line
  2020-10-20 20:19       ` Junio C Hamano
@ 2020-10-21 13:16         ` Charvi Mendiratta
  0 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-21 13:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Christian Couder, phillip.wood123,
	Đoàn Trần Công Danh, Taylor Blau

On Wed, 21 Oct 2020 at 01:49, Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Charvi Mendiratta <charvi077@gmail.com> writes:
> >
> >> Modern practice is to avoid multiple commands per line,
> >> and instead place each command on its own line.
> >>
> >> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> >> ---
> >
> > This looks good, but I am wondering what happened between v3 and
> > v4.
> >
> > As you've demonstrated through the microproject that you can now
> > comfortably be involved in the review discussion, I am tempted to
> > suggest that we declare victory at this point and move on, but I
> > don't know what the plans are for the other 4 patches (I guess we
> > won't miss them that much---the micros are meant to be practice
> > targets).
>
> Actually I take it back.  This does not look good as a standalone
> patch at all.  It seems to depend on something in the 5-patch
> series.
>

Yes, Thanks a lot Junio. I totally agree after applying it locally and
getting stuck in am conflicts without using v3.

> Please make sure that patches you send are usable by your
> recipients.
>
I will take this as an important note. I have fixed it and sent the patch
series after testing it locally .

Thanks and Regards,
Charvi

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-21 12:48       ` [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
@ 2020-10-21 17:20         ` Eric Sunshine
  2020-10-22  5:44           ` Junio C Hamano
  0 siblings, 1 reply; 60+ messages in thread
From: Eric Sunshine @ 2020-10-21 17:20 UTC (permalink / raw)
  To: Charvi Mendiratta
  Cc: Git List, Junio C Hamano, Christian Couder, Taylor Blau, Phillip Wood

On Wed, Oct 21, 2020 at 8:51 AM Charvi Mendiratta <charvi077@gmail.com> wrote:
> According to Documentation/CodingGuidelines, redirect
> operator is written with space before, but no space
> after them.
>
> Let's remove these whitespaces after redirect operators.
>
> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> ---
> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> @@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
> -cat > expect << EOF
> +cat >expect << EOF

There's still a space after the '<<' operator which should be removed:

    cat >expect <<EOF

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-21 17:20         ` Eric Sunshine
@ 2020-10-22  5:44           ` Junio C Hamano
  2020-10-22  5:53             ` Eric Sunshine
                               ` (2 more replies)
  0 siblings, 3 replies; 60+ messages in thread
From: Junio C Hamano @ 2020-10-22  5:44 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Charvi Mendiratta, Git List, Christian Couder, Taylor Blau, Phillip Wood

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Oct 21, 2020 at 8:51 AM Charvi Mendiratta <charvi077@gmail.com> wrote:
>> According to Documentation/CodingGuidelines, redirect
>> operator is written with space before, but no space
>> after them.
>>
>> Let's remove these whitespaces after redirect operators.
>>
>> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
>> ---
>> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
>> @@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
>> -cat > expect << EOF
>> +cat >expect << EOF
>
> There's still a space after the '<<' operator which should be removed:
>
>     cat >expect <<EOF

Also, this is outside any test_expect_*, which is unusual in
modernized test scripts.  There are many other instances of
preparing expected output outside test_expect_* in this file,
so we may need another patch to clean them up.

For now, within the context of this patch, let's just fix the space
after the << here-doc redirection operator, as you spotted.  The
attached I'll squash into this patch.

Thanks.

 t/t7102-reset.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git i/t/t7102-reset.sh w/t/t7102-reset.sh
index a8c96bf162..07acaa2beb 100755
--- i/t/t7102-reset.sh
+++ w/t/t7102-reset.sh
@@ -460,7 +460,7 @@ test_expect_success 'resetting an unmodified path is a no-op' '
 	git diff-index --cached --exit-code HEAD
 '
 
-cat >expect << EOF
+cat >expect <<EOF
 Unstaged changes after reset:
 M	file2
 EOF




^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-22  5:44           ` Junio C Hamano
@ 2020-10-22  5:53             ` Eric Sunshine
  2020-10-22  5:55             ` Junio C Hamano
  2020-10-22  6:29             ` Charvi Mendiratta
  2 siblings, 0 replies; 60+ messages in thread
From: Eric Sunshine @ 2020-10-22  5:53 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Charvi Mendiratta, Git List, Christian Couder, Taylor Blau, Phillip Wood

On Thu, Oct 22, 2020 at 1:44 AM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > There's still a space after the '<<' operator which should be removed:
> >
> >     cat >expect <<EOF
>
> Also, this is outside any test_expect_*, which is unusual in
> modernized test scripts.  There are many other instances of
> preparing expected output outside test_expect_* in this file,
> so we may need another patch to clean them up.

I noticed that, as well, but wasn't sure if mentioning it was
worthwhile considering that this is a microproject and we're already
at v5. It can certainly be addressed by someone as a follow-on patch
if desired.

> For now, within the context of this patch, let's just fix the space
> after the << here-doc redirection operator, as you spotted.  The
> attached I'll squash into this patch.
>
> -cat >expect << EOF
> +cat >expect <<EOF

Looks fine.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-22  5:44           ` Junio C Hamano
  2020-10-22  5:53             ` Eric Sunshine
@ 2020-10-22  5:55             ` Junio C Hamano
  2020-10-22  6:04               ` Eric Sunshine
  2020-10-22  6:29             ` Charvi Mendiratta
  2 siblings, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2020-10-22  5:55 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Charvi Mendiratta, Git List, Christian Couder, Taylor Blau, Phillip Wood

Junio C Hamano <gitster@pobox.com> writes:

> Also, this is outside any test_expect_*, which is unusual in
> modernized test scripts.  There are many other instances of
> preparing expected output outside test_expect_* in this file,
> so we may need another patch to clean them up.
>
> For now, within the context of this patch, let's just fix the space
> after the << here-doc redirection operator, as you spotted.  The
> attached I'll squash into this patch.

And the other clean-up patch would look like this.

--- >8 ---
Subject: t7102: prepare expected output inside test_expect_* block

That way we can notice if there is a breakage/bug in the parts of
the test that prepare the expected outcome, which is how modern
tests are arranged.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7102-reset.sh | 244 +++++++++++++++++++++++++++----------------------------
 1 file changed, 121 insertions(+), 123 deletions(-)

diff --git c/t/t7102-reset.sh w/t/t7102-reset.sh
index 07acaa2beb..821e8bb94d 100755
--- c/t/t7102-reset.sh
+++ w/t/t7102-reset.sh
@@ -82,15 +82,15 @@ test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' '
 	test_i18ncmp .expected .actual
 '
 
->.diff_expect
->.cached_expect
-cat >.cat_expect <<EOF
-secondfile:
-1st line 2nd file
-2nd line 2nd file
-EOF
-
 test_expect_success 'giving a non existing revision should fail' '
+	>.diff_expect &&
+	>.cached_expect &&
+	cat >.cat_expect <<-\EOF &&
+	secondfile:
+	1st line 2nd file
+	2nd line 2nd file
+	EOF
+
 	test_must_fail git reset aaaaaa &&
 	test_must_fail git reset --mixed aaaaaa &&
 	test_must_fail git reset --soft aaaaaa &&
@@ -191,38 +191,38 @@ test_expect_success 'resetting to HEAD with no changes should succeed and do not
 		check_changes $head5
 '
 
->.diff_expect
-cat >.cached_expect <<EOF
-diff --git a/secondfile b/secondfile
-index $head5p1s..$head5s 100644
---- a/secondfile
-+++ b/secondfile
-@@ -1 +1,2 @@
--2nd file
-+1st line 2nd file
-+2nd line 2nd file
-EOF
-cat >.cat_expect <<EOF
-secondfile:
-1st line 2nd file
-2nd line 2nd file
-EOF
 test_expect_success '--soft reset only should show changes in diff --cached' '
+	>.diff_expect &&
+	cat >.cached_expect <<-EOF &&
+	diff --git a/secondfile b/secondfile
+	index $head5p1s..$head5s 100644
+	--- a/secondfile
+	+++ b/secondfile
+	@@ -1 +1,2 @@
+	-2nd file
+	+1st line 2nd file
+	+2nd line 2nd file
+	EOF
+	cat >.cat_expect <<-\EOF &&
+	secondfile:
+	1st line 2nd file
+	2nd line 2nd file
+	EOF
 	git reset --soft HEAD^ &&
 	check_changes $head5p1 &&
 	test "$(git rev-parse ORIG_HEAD)" = \
 			$head5
 '
 
->.diff_expect
->.cached_expect
-cat >.cat_expect <<EOF
-secondfile:
-1st line 2nd file
-2nd line 2nd file
-3rd line 2nd file
-EOF
 test_expect_success 'changing files and redo the last commit should succeed' '
+	>.diff_expect &&
+	>.cached_expect &&
+	cat >.cat_expect <<-\EOF &&
+	secondfile:
+	1st line 2nd file
+	2nd line 2nd file
+	3rd line 2nd file
+	EOF
 	echo "3rd line 2nd file" >>secondfile &&
 	git commit -a -C ORIG_HEAD &&
 	head4=$(git rev-parse --verify HEAD) &&
@@ -231,54 +231,54 @@ test_expect_success 'changing files and redo the last commit should succeed' '
 			$head5
 '
 
->.diff_expect
->.cached_expect
-cat >.cat_expect <<EOF
-first:
-1st file
-2nd line 1st file
-second:
-2nd file
-EOF
 test_expect_success '--hard reset should change the files and undo commits permanently' '
+	>.diff_expect &&
+	>.cached_expect &&
+	cat >.cat_expect <<-\EOF &&
+	first:
+	1st file
+	2nd line 1st file
+	second:
+	2nd file
+	EOF
 	git reset --hard HEAD~2 &&
 	check_changes $head5p2 &&
 	test "$(git rev-parse ORIG_HEAD)" = \
 			$head4
 '
 
->.diff_expect
-cat >.cached_expect <<EOF
-diff --git a/first b/first
-deleted file mode 100644
-index $head5p2f..0000000
---- a/first
-+++ /dev/null
-@@ -1,2 +0,0 @@
--1st file
--2nd line 1st file
-diff --git a/second b/second
-deleted file mode 100644
-index $head5p1s..0000000
---- a/second
-+++ /dev/null
-@@ -1 +0,0 @@
--2nd file
-diff --git a/secondfile b/secondfile
-new file mode 100644
-index 0000000..$head5s
---- /dev/null
-+++ b/secondfile
-@@ -0,0 +1,2 @@
-+1st line 2nd file
-+2nd line 2nd file
-EOF
-cat >.cat_expect <<EOF
-secondfile:
-1st line 2nd file
-2nd line 2nd file
-EOF
 test_expect_success 'redoing changes adding them without commit them should succeed' '
+	>.diff_expect &&
+	cat >.cached_expect <<-EOF &&
+	diff --git a/first b/first
+	deleted file mode 100644
+	index $head5p2f..0000000
+	--- a/first
+	+++ /dev/null
+	@@ -1,2 +0,0 @@
+	-1st file
+	-2nd line 1st file
+	diff --git a/second b/second
+	deleted file mode 100644
+	index $head5p1s..0000000
+	--- a/second
+	+++ /dev/null
+	@@ -1 +0,0 @@
+	-2nd file
+	diff --git a/secondfile b/secondfile
+	new file mode 100644
+	index 0000000..$head5s
+	--- /dev/null
+	+++ b/secondfile
+	@@ -0,0 +1,2 @@
+	+1st line 2nd file
+	+2nd line 2nd file
+	EOF
+	cat >.cat_expect <<-\EOF &&
+	secondfile:
+	1st line 2nd file
+	2nd line 2nd file
+	EOF
 	git rm first &&
 	git mv second secondfile &&
 
@@ -288,46 +288,45 @@ test_expect_success 'redoing changes adding them without commit them should succ
 	check_changes $head5p2
 '
 
-cat >.diff_expect <<EOF
-diff --git a/first b/first
-deleted file mode 100644
-index $head5p2f..0000000
---- a/first
-+++ /dev/null
-@@ -1,2 +0,0 @@
--1st file
--2nd line 1st file
-diff --git a/second b/second
-deleted file mode 100644
-index $head5p1s..0000000
---- a/second
-+++ /dev/null
-@@ -1 +0,0 @@
--2nd file
-EOF
->.cached_expect
-cat >.cat_expect <<EOF
-secondfile:
-1st line 2nd file
-2nd line 2nd file
-EOF
 test_expect_success '--mixed reset to HEAD should unadd the files' '
+	cat >.diff_expect <<-EOF &&
+	diff --git a/first b/first
+	deleted file mode 100644
+	index $head5p2f..0000000
+	--- a/first
+	+++ /dev/null
+	@@ -1,2 +0,0 @@
+	-1st file
+	-2nd line 1st file
+	diff --git a/second b/second
+	deleted file mode 100644
+	index $head5p1s..0000000
+	--- a/second
+	+++ /dev/null
+	@@ -1 +0,0 @@
+	-2nd file
+	EOF
+	>.cached_expect &&
+	cat >.cat_expect <<-\EOF &&
+	secondfile:
+	1st line 2nd file
+	2nd line 2nd file
+	EOF
 	git reset &&
 	check_changes $head5p2 &&
 	test "$(git rev-parse ORIG_HEAD)" = $head5p2
 '
 
->.diff_expect
->.cached_expect
-cat >.cat_expect <<EOF
-secondfile:
-1st line 2nd file
-2nd line 2nd file
-EOF
 test_expect_success 'redoing the last two commits should succeed' '
+	>.diff_expect &&
+	>.cached_expect &&
+	cat >.cat_expect <<-\EOF &&
+	secondfile:
+	1st line 2nd file
+	2nd line 2nd file
+	EOF
 	git add secondfile &&
 	git reset --hard $head5p2 &&
-
 	git rm first &&
 	git mv second secondfile &&
 	git commit -a -m "remove 1st and rename 2nd" &&
@@ -340,15 +339,15 @@ test_expect_success 'redoing the last two commits should succeed' '
 	check_changes $head5
 '
 
->.diff_expect
->.cached_expect
-cat >.cat_expect <<EOF
-secondfile:
-1st line 2nd file
-2nd line 2nd file
-3rd line in branch2
-EOF
 test_expect_success '--hard reset to HEAD should clear a failed merge' '
+	>.diff_expect &&
+	>.cached_expect &&
+	cat >.cat_expect <<-\EOF &&
+	secondfile:
+	1st line 2nd file
+	2nd line 2nd file
+	3rd line in branch2
+	EOF
 	git branch branch1 &&
 	git branch branch2 &&
 
@@ -366,14 +365,14 @@ test_expect_success '--hard reset to HEAD should clear a failed merge' '
 	check_changes $head3
 '
 
->.diff_expect
->.cached_expect
-cat >.cat_expect <<EOF
-secondfile:
-1st line 2nd file
-2nd line 2nd file
-EOF
 test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge' '
+	>.diff_expect &&
+	>.cached_expect &&
+	cat >.cat_expect <<-\EOF &&
+	secondfile:
+	1st line 2nd file
+	2nd line 2nd file
+	EOF
 	git reset --hard HEAD^ &&
 	check_changes $head5 &&
 
@@ -460,12 +459,11 @@ test_expect_success 'resetting an unmodified path is a no-op' '
 	git diff-index --cached --exit-code HEAD
 '
 
-cat >expect <<EOF
-Unstaged changes after reset:
-M	file2
-EOF
-
 test_expect_success '--mixed refreshes the index' '
+	cat >expect <<-\EOF &&
+	Unstaged changes after reset:
+	M	file2
+	EOF
 	echo 123 >> file2 &&
 	git reset --mixed HEAD >output &&
 	test_i18ncmp expect output

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-22  5:55             ` Junio C Hamano
@ 2020-10-22  6:04               ` Eric Sunshine
  2020-10-22 17:35                 ` Junio C Hamano
  0 siblings, 1 reply; 60+ messages in thread
From: Eric Sunshine @ 2020-10-22  6:04 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Charvi Mendiratta, Git List, Christian Couder, Taylor Blau, Phillip Wood

On Thu, Oct 22, 2020 at 1:56 AM Junio C Hamano <gitster@pobox.com> wrote:
> Subject: t7102: prepare expected output inside test_expect_* block
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git c/t/t7102-reset.sh w/t/t7102-reset.sh
> @@ -82,15 +82,15 @@ test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' '
> ->.diff_expect
> ->.cached_expect
> -cat >.cat_expect <<EOF
> -secondfile:
> -EOF
> -
>  test_expect_success 'giving a non existing revision should fail' '
> +       >.diff_expect &&
> +       >.cached_expect &&
> +       cat >.cat_expect <<-\EOF &&
> +       secondfile:
> +       EOF

You used <<-\EOF rather than plain <<-EOF when possible. Good.

(Might be worth mention in the commit message, but perhaps too minor?)

> @@ -191,38 +191,38 @@ test_expect_success 'resetting to HEAD with no changes should succeed and do not
>  test_expect_success '--soft reset only should show changes in diff --cached' '
> +       >.diff_expect &&
> +       cat >.cached_expect <<-EOF &&
> +       diff --git a/secondfile b/secondfile
> +       index $head5p1s..$head5s 100644

And used plain <<-EOF when necessary. Fine.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-22  5:44           ` Junio C Hamano
  2020-10-22  5:53             ` Eric Sunshine
  2020-10-22  5:55             ` Junio C Hamano
@ 2020-10-22  6:29             ` Charvi Mendiratta
  2 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-22  6:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Sunshine, Git List, Christian Couder, Taylor Blau, Phillip Wood

On Thu, 22 Oct 2020 at 11:14, Junio C Hamano <gitster@pobox.com> wrote:
>
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
> > On Wed, Oct 21, 2020 at 8:51 AM Charvi Mendiratta <charvi077@gmail.com> wrote:
> >> According to Documentation/CodingGuidelines, redirect
> >> operator is written with space before, but no space
> >> after them.
> >>
> >> Let's remove these whitespaces after redirect operators.
> >>
> >> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> >> ---
> >> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> >> @@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
> >> -cat > expect << EOF
> >> +cat >expect << EOF
> >
> > There's still a space after the '<<' operator which should be removed:
> >
> >     cat >expect <<EOF
>
> Also, this is outside any test_expect_*, which is unusual in
> modernized test scripts.  There are many other instances of
> preparing expected output outside test_expect_* in this file,
> so we may need another patch to clean them up.
>
> For now, within the context of this patch, let's just fix the space
> after the << here-doc redirection operator, as you spotted.  The
> attached I'll squash into this patch.
>
> Thanks.
>

Thanks Eric and Junio, I am resending the patch with the fix of
space after <<  as mentioned.

>  t/t7102-reset.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git i/t/t7102-reset.sh w/t/t7102-reset.sh
> index a8c96bf162..07acaa2beb 100755
> --- i/t/t7102-reset.sh
> +++ w/t/t7102-reset.sh
> @@ -460,7 +460,7 @@ test_expect_success 'resetting an unmodified path is a no-op' '
>         git diff-index --cached --exit-code HEAD
>  '
>
> -cat >expect << EOF
> +cat >expect <<EOF
>  Unstaged changes after reset:
>  M      file2
>  EOF
>
>
>

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 0/5][Outreachy] modernize test scripts
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                         ` (4 preceding siblings ...)
  2020-10-21 12:48       ` [PATCH v5 5/5][Outreachy] t7201: put each command on a separate line Charvi Mendiratta
@ 2020-10-22  7:16       ` Charvi Mendiratta
  2020-10-22  7:16       ` [PATCH v6 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
                         ` (4 subsequent siblings)
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-22  7:16 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, me, phillip.wood123, sunshine,
	Charvi Mendiratta

In this patch series, I have fixed the remaining errors of 
whitespace after redirect operator in t7102 test script and
v5 3/5 is updated. Other patches remains unchanged.

This patch series :
 -modernize the three test scripts : t7101, t7201 and t7102.
 - cleans up with 5 types of changes in all the three scripts.
   1. Converting the old style test format to new one
   2. Removing blankspaces in test bodies
   3. Removing whitespaces after the redirect operator, according to
      CodingGuidelines 
   4. Using 'git -C' to avoid use of another subshell 
   5. Placing commands in separate lines



Charvi Mendiratta (5):
  t7101,t7102,t7201: modernize test formatting
  t7102,t7201: remove unnecessary blank spaces in test body
  t7102,t7201: remove whitespace after redirect operator
  t7201: use 'git -C' to avoid subshell
  t7201: put each command on a separate line

 t/t7101-reset-empty-subdirs.sh |  66 ++++++++++-----------
 t/t7102-reset.sh               |  65 ++++++++-------------
 t/t7201-co.sh                  | 102 +++++++++++++--------------------
 3 files changed, 97 insertions(+), 136 deletions(-)

-- 
2.29.0.rc1


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                         ` (5 preceding siblings ...)
  2020-10-22  7:16       ` [PATCH v6 0/5][Outreachy] modernize test scripts Charvi Mendiratta
@ 2020-10-22  7:16       ` Charvi Mendiratta
  2020-10-22  7:16       ` [PATCH v6 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
                         ` (3 subsequent siblings)
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-22  7:16 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, me, phillip.wood123, sunshine,
	Charvi Mendiratta

Some tests in these scripts are formatted using a very old style:
        test_expect_success \
            'title' \
            'body line 1 &&
             body line 2'

Updating the formatting to the modern style:
        test_expect_success 'title' '
            body line 1 &&
            body line 2
        '

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7101-reset-empty-subdirs.sh | 66 +++++++++++++++++-----------------
 t/t7102-reset.sh               | 24 +++++--------
 t/t7201-co.sh                  | 31 ++++++++--------
 3 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/t/t7101-reset-empty-subdirs.sh b/t/t7101-reset-empty-subdirs.sh
index 96e163f084..bfce05ac5d 100755
--- a/t/t7101-reset-empty-subdirs.sh
+++ b/t/t7101-reset-empty-subdirs.sh
@@ -6,16 +6,15 @@
 test_description='git reset should cull empty subdirs'
 . ./test-lib.sh
 
-test_expect_success \
-    'creating initial files' \
-    'mkdir path0 &&
+test_expect_success 'creating initial files' '
+     mkdir path0 &&
      cp "$TEST_DIRECTORY"/../COPYING path0/COPYING &&
      git add path0/COPYING &&
-     git commit -m add -a'
+     git commit -m add -a
+'
 
-test_expect_success \
-    'creating second files' \
-    'mkdir path1 &&
+test_expect_success 'creating second files' '
+     mkdir path1 &&
      mkdir path1/path2 &&
      cp "$TEST_DIRECTORY"/../COPYING path1/path2/COPYING &&
      cp "$TEST_DIRECTORY"/../COPYING path1/COPYING &&
@@ -25,39 +24,40 @@ test_expect_success \
      git add path1/COPYING &&
      git add COPYING &&
      git add path0/COPYING-TOO &&
-     git commit -m change -a'
+     git commit -m change -a
+'
 
-test_expect_success \
-    'resetting tree HEAD^' \
-    'git reset --hard HEAD^'
+test_expect_success 'resetting tree HEAD^' '
+     git reset --hard HEAD^
+'
 
-test_expect_success \
-    'checking initial files exist after rewind' \
-    'test -d path0 &&
-     test -f path0/COPYING'
+test_expect_success 'checking initial files exist after rewind' '
+     test -d path0 &&
+     test -f path0/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/path2/COPYING' \
-    '! test -f path1/path2/COPYING'
+test_expect_success 'checking lack of path1/path2/COPYING' '
+    ! test -f path1/path2/COPYING
+'
 
-test_expect_success \
-    'checking lack of path1/COPYING' \
-    '! test -f path1/COPYING'
+test_expect_success 'checking lack of path1/COPYING' '
+    ! test -f path1/COPYING
+'
 
-test_expect_success \
-    'checking lack of COPYING' \
-    '! test -f COPYING'
+test_expect_success 'checking lack of COPYING' '
+     ! test -f COPYING
+'
 
-test_expect_success \
-    'checking checking lack of path1/COPYING-TOO' \
-    '! test -f path0/COPYING-TOO'
+test_expect_success 'checking checking lack of path1/COPYING-TOO' '
+     ! test -f path0/COPYING-TOO
+'
 
-test_expect_success \
-    'checking lack of path1/path2' \
-    '! test -d path1/path2'
+test_expect_success 'checking lack of path1/path2' '
+     ! test -d path1/path2
+'
 
-test_expect_success \
-    'checking lack of path1' \
-    '! test -d path1'
+test_expect_success 'checking lack of path1' '
+     ! test -d path1
+'
 
 test_done
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 22161b3b2d..fe43f77513 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -107,8 +107,7 @@ test_expect_success 'reset --soft with unmerged index should fail' '
 	git rm --cached -- un
 '
 
-test_expect_success \
-	'giving paths with options different than --mixed should fail' '
+test_expect_success 'giving paths with options different than --mixed should fail' '
 	test_must_fail git reset --soft -- first &&
 	test_must_fail git reset --hard -- first &&
 	test_must_fail git reset --soft HEAD^ -- first &&
@@ -128,8 +127,7 @@ test_expect_success 'giving unrecognized options should fail' '
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending merge should fail' '
+test_expect_success 'trying to do reset --soft with pending merge should fail' '
 	git branch branch1 &&
 	git branch branch2 &&
 
@@ -152,8 +150,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'trying to do reset --soft with pending checkout merge should fail' '
+test_expect_success 'trying to do reset --soft with pending checkout merge should fail' '
 	git branch branch3 &&
 	git branch branch4 &&
 
@@ -175,8 +172,7 @@ test_expect_success \
 	check_changes $head5
 '
 
-test_expect_success \
-	'resetting to HEAD with no changes should succeed and do nothing' '
+test_expect_success 'resetting to HEAD with no changes should succeed and do nothing' '
 	git reset --hard &&
 		check_changes $head5 &&
 	git reset --hard HEAD &&
@@ -226,8 +222,7 @@ secondfile:
 2nd line 2nd file
 3rd line 2nd file
 EOF
-test_expect_success \
-	'changing files and redo the last commit should succeed' '
+test_expect_success 'changing files and redo the last commit should succeed' '
 	echo "3rd line 2nd file" >>secondfile &&
 	git commit -a -C ORIG_HEAD &&
 	head4=$(git rev-parse --verify HEAD) &&
@@ -245,8 +240,7 @@ first:
 second:
 2nd file
 EOF
-test_expect_success \
-	'--hard reset should change the files and undo commits permanently' '
+test_expect_success '--hard reset should change the files and undo commits permanently' '
 	git reset --hard HEAD~2 &&
 	check_changes $head5p2 &&
 	test "$(git rev-parse ORIG_HEAD)" = \
@@ -284,8 +278,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'redoing changes adding them without commit them should succeed' '
+test_expect_success 'redoing changes adding them without commit them should succeed' '
 	git rm first &&
 	git mv second secondfile &&
 
@@ -380,8 +373,7 @@ secondfile:
 1st line 2nd file
 2nd line 2nd file
 EOF
-test_expect_success \
-	'--hard reset to ORIG_HEAD should clear a fast-forward merge' '
+test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge' '
 	git reset --hard HEAD^ &&
 	check_changes $head5 &&
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 4d62b9b00f..a800bda5e3 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -62,7 +62,7 @@ test_expect_success setup '
 	git checkout master
 '
 
-test_expect_success "checkout from non-existing branch" '
+test_expect_success 'checkout from non-existing branch' '
 
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
@@ -71,7 +71,7 @@ test_expect_success "checkout from non-existing branch" '
 	test refs/heads/master = "$(git symbolic-ref HEAD)"
 '
 
-test_expect_success "checkout with dirty tree without -m" '
+test_expect_success 'checkout with dirty tree without -m' '
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
@@ -84,7 +84,7 @@ test_expect_success "checkout with dirty tree without -m" '
 
 '
 
-test_expect_success "checkout with unrelated dirty tree without -m" '
+test_expect_success 'checkout with unrelated dirty tree without -m' '
 
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
@@ -95,7 +95,7 @@ test_expect_success "checkout with unrelated dirty tree without -m" '
 	test_cmp messages.expect messages
 '
 
-test_expect_success "checkout -m with dirty tree" '
+test_expect_success 'checkout -m with dirty tree' '
 
 	git checkout -f master &&
 	git clean -f &&
@@ -120,7 +120,7 @@ test_expect_success "checkout -m with dirty tree" '
 	test_must_be_empty current.index
 '
 
-test_expect_success "checkout -m with dirty tree, renamed" '
+test_expect_success 'checkout -m with dirty tree, renamed' '
 
 	git checkout -f master && git clean -f &&
 
@@ -388,22 +388,22 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 '
 
-test_expect_success \
-    'checkout w/--track sets up tracking' '
+test_expect_success 'checkout w/--track sets up tracking' '
     git config branch.autosetupmerge false &&
     git checkout master &&
     git checkout --track -b track1 &&
     test "$(git config branch.track1.remote)" &&
-    test "$(git config branch.track1.merge)"'
+    test "$(git config branch.track1.merge)"
+'
 
-test_expect_success \
-    'checkout w/autosetupmerge=always sets up tracking' '
+test_expect_success 'checkout w/autosetupmerge=always sets up tracking' '
     test_when_finished git config branch.autosetupmerge false &&
     git config branch.autosetupmerge always &&
     git checkout master &&
     git checkout -b track2 &&
     test "$(git config branch.track2.remote)" &&
-    test "$(git config branch.track2.merge)"'
+    test "$(git config branch.track2.merge)"
+'
 
 test_expect_success 'checkout w/--track from non-branch HEAD fails' '
     git checkout master^0 &&
@@ -435,8 +435,7 @@ test_expect_success 'detach a symbolic link HEAD' '
     test "z$(git rev-parse --verify refs/heads/master)" = "z$here"
 '
 
-test_expect_success \
-    'checkout with --track fakes a sensible -b <name>' '
+test_expect_success 'checkout with --track fakes a sensible -b <name>' '
     git config remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" &&
     git update-ref refs/remotes/origin/koala/bear renamer &&
 
@@ -457,9 +456,9 @@ test_expect_success \
     test "$(git rev-parse HEAD)" = "$(git rev-parse renamer)"
 '
 
-test_expect_success \
-    'checkout with --track, but without -b, fails with too short tracked name' '
-    test_must_fail git checkout --track renamer'
+test_expect_success 'checkout with --track, but without -b, fails with too short tracked name' '
+    test_must_fail git checkout --track renamer
+'
 
 setup_conflicting_index () {
 	rm -f .git/index &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                         ` (6 preceding siblings ...)
  2020-10-22  7:16       ` [PATCH v6 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
@ 2020-10-22  7:16       ` Charvi Mendiratta
  2020-10-22  7:16       ` [PATCH v6 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
                         ` (2 subsequent siblings)
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-22  7:16 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, me, phillip.wood123, sunshine,
	Charvi Mendiratta

t7102 and t7201 still follow the old style of having blank
lines around test body, which is not consistence with our
current practice.

Let's remove those unnecessary blank lines.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7102-reset.sh |  9 ---------
 t/t7201-co.sh    | 25 -------------------------
 2 files changed, 34 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index fe43f77513..2b4cfb2c83 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -439,7 +439,6 @@ test_expect_success 'test --mixed <paths>' '
 '
 
 test_expect_success 'test resetting the index at give paths' '
-
 	mkdir sub &&
 	>sub/file1 &&
 	>sub/file2 &&
@@ -452,7 +451,6 @@ test_expect_success 'test resetting the index at give paths' '
 	echo "$U" &&
 	test_must_fail git diff-index --cached --exit-code "$T" &&
 	test "$T" != "$U"
-
 '
 
 test_expect_success 'resetting an unmodified path is a no-op' '
@@ -490,7 +488,6 @@ test_expect_success 'resetting specific path that is unmerged' '
 '
 
 test_expect_success 'disambiguation (1)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -499,11 +496,9 @@ test_expect_success 'disambiguation (1)' '
 	test -z "$(git diff --cached --name-only)" &&
 	test -f secondfile &&
 	test_must_be_empty secondfile
-
 '
 
 test_expect_success 'disambiguation (2)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -511,11 +506,9 @@ test_expect_success 'disambiguation (2)' '
 	test_must_fail git reset secondfile &&
 	test -n "$(git diff --cached --name-only -- secondfile)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (3)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
@@ -524,11 +517,9 @@ test_expect_success 'disambiguation (3)' '
 	test_must_fail git diff --quiet &&
 	test -z "$(git diff --cached --name-only)" &&
 	test ! -f secondfile
-
 '
 
 test_expect_success 'disambiguation (4)' '
-
 	git reset --hard &&
 	>secondfile &&
 	git add secondfile &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index a800bda5e3..b527f8009c 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,6 @@ fill () {
 
 
 test_expect_success setup '
-
 	fill x y z > same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
@@ -63,7 +62,6 @@ test_expect_success setup '
 '
 
 test_expect_success 'checkout from non-existing branch' '
-
 	git checkout -b delete-me master &&
 	git update-ref -d --no-deref refs/heads/delete-me &&
 	test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
@@ -72,7 +70,6 @@ test_expect_success 'checkout from non-existing branch' '
 '
 
 test_expect_success 'checkout with dirty tree without -m' '
-
 	fill 0 1 2 3 4 5 6 7 8 >one &&
 	if git checkout side
 	then
@@ -81,11 +78,9 @@ test_expect_success 'checkout with dirty tree without -m' '
 	else
 		echo "happy - failed correctly"
 	fi
-
 '
 
 test_expect_success 'checkout with unrelated dirty tree without -m' '
-
 	git checkout -f master &&
 	fill 0 1 2 3 4 5 6 7 8 >same &&
 	cp same kept &&
@@ -96,7 +91,6 @@ test_expect_success 'checkout with unrelated dirty tree without -m' '
 '
 
 test_expect_success 'checkout -m with dirty tree' '
-
 	git checkout -f master &&
 	git clean -f &&
 
@@ -121,7 +115,6 @@ test_expect_success 'checkout -m with dirty tree' '
 '
 
 test_expect_success 'checkout -m with dirty tree, renamed' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 2 3 4 5 7 8 >one &&
@@ -139,11 +132,9 @@ test_expect_success 'checkout -m with dirty tree, renamed' '
 	! test -f one &&
 	git diff --cached >current &&
 	test_must_be_empty current
-
 '
 
 test_expect_success 'checkout -m with merge conflict' '
-
 	git checkout -f master && git clean -f &&
 
 	fill 1 T 3 4 5 6 S 8 >one &&
@@ -166,7 +157,6 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-
 	git checkout -f master && git clean -f &&
 
 	fill b d > two &&
@@ -190,7 +180,6 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 
 	fill b d > two &&
@@ -216,7 +205,6 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-
 	git checkout -f master && git reset --hard && git clean -f &&
 	git rm two &&
 
@@ -228,7 +216,6 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 '
 
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
-
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
 	git checkout -f renamer && git clean -f &&
@@ -267,7 +254,6 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-
 	git checkout -f master && git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -283,7 +269,6 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-
 	git checkout -f master && git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -299,7 +284,6 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-
 	git checkout -f master && git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
@@ -315,7 +299,6 @@ test_expect_success 'checkout to detach HEAD with HEAD^0' '
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git tag both side &&
 	git branch both master &&
 	git reset --hard &&
@@ -327,11 +310,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	test "z$H" = "z$M" &&
 	name=$(git symbolic-ref HEAD 2>/dev/null) &&
 	test "z$name" = zrefs/heads/both
-
 '
 
 test_expect_success 'checkout with ambiguous tag/branch names' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -351,11 +332,9 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 	else
 		: happy
 	fi
-
 '
 
 test_expect_success 'switch branches while in subdirectory' '
-
 	git reset --hard &&
 	git checkout master &&
 
@@ -366,11 +345,9 @@ test_expect_success 'switch branches while in subdirectory' '
 	) &&
 	! test -f subs/one &&
 	rm -fr subs
-
 '
 
 test_expect_success 'checkout specific path while in subdirectory' '
-
 	git reset --hard &&
 	git checkout side &&
 	mkdir subs &&
@@ -385,7 +362,6 @@ test_expect_success 'checkout specific path while in subdirectory' '
 		git checkout side -- bero
 	) &&
 	test -f subs/bero
-
 '
 
 test_expect_success 'checkout w/--track sets up tracking' '
@@ -608,7 +584,6 @@ test_expect_success 'failing checkout -b should not break working tree' '
 	test $(git symbolic-ref HEAD) = refs/heads/master &&
 	git diff --exit-code &&
 	git diff --cached --exit-code
-
 '
 
 test_expect_success 'switch out of non-branch' '
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                         ` (7 preceding siblings ...)
  2020-10-22  7:16       ` [PATCH v6 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
@ 2020-10-22  7:16       ` Charvi Mendiratta
  2020-10-22  7:16       ` [PATCH v6 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
  2020-10-22  7:16       ` [PATCH v6 5/5][Outreachy] t7201: put each command on a separate line Charvi Mendiratta
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-22  7:16 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, me, phillip.wood123, sunshine,
	Charvi Mendiratta

According to Documentation/CodingGuidelines, redirect
operator is written with space before, but no space
after them.

Let's remove these whitespaces after redirect operators.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7102-reset.sh | 32 ++++++++++++++++----------------
 t/t7201-co.sh    | 10 +++++-----
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 2b4cfb2c83..3d190cad0e 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -70,15 +70,15 @@ check_changes () {
 
 test_expect_success 'reset --hard message' '
 	hex=$(git log -1 --format="%h") &&
-	git reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg) > .expected &&
+	git reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
 test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' '
 	hex=$(git log -1 --format="%h") &&
-	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard > .actual &&
-	echo HEAD is now at $hex $(commit_msg $test_encoding) > .expected &&
+	git -c "i18n.logOutputEncoding=$test_encoding" reset --hard >.actual &&
+	echo HEAD is now at $hex $(commit_msg $test_encoding) >.expected &&
 	test_i18ncmp .expected .actual
 '
 
@@ -387,25 +387,25 @@ test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge
 '
 
 test_expect_success 'test --mixed <paths>' '
-	echo 1 > file1 &&
-	echo 2 > file2 &&
+	echo 1 >file1 &&
+	echo 2 >file2 &&
 	git add file1 file2 &&
 	test_tick &&
 	git commit -m files &&
 	before1=$(git rev-parse --short HEAD:file1) &&
 	before2=$(git rev-parse --short HEAD:file2) &&
 	git rm file2 &&
-	echo 3 > file3 &&
-	echo 4 > file4 &&
-	echo 5 > file1 &&
+	echo 3 >file3 &&
+	echo 4 >file4 &&
+	echo 5 >file1 &&
 	after1=$(git rev-parse --short $(git hash-object file1)) &&
 	after4=$(git rev-parse --short $(git hash-object file4)) &&
 	git add file1 file3 file4 &&
 	git reset HEAD -- file1 file2 file3 &&
 	test_must_fail git diff --quiet &&
-	git diff > output &&
+	git diff >output &&
 
-	cat > expect <<-EOF &&
+	cat >expect <<-EOF &&
 	diff --git a/file1 b/file1
 	index $before1..$after1 100644
 	--- a/file1
@@ -423,9 +423,9 @@ test_expect_success 'test --mixed <paths>' '
 	EOF
 
 	test_cmp expect output &&
-	git diff --cached > output &&
+	git diff --cached >output &&
 
-	cat > cached_expect <<-EOF &&
+	cat >cached_expect <<-EOF &&
 	diff --git a/file4 b/file4
 	new file mode 100644
 	index 0000000..$after4
@@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
 	git diff-index --cached --exit-code HEAD
 '
 
-cat > expect << EOF
+cat >expect <<EOF
 Unstaged changes after reset:
 M	file2
 EOF
 
 test_expect_success '--mixed refreshes the index' '
-	echo 123 >> file2 &&
-	git reset --mixed HEAD > output &&
+	echo 123 >>file2 &&
+	git reset --mixed HEAD >output &&
 	test_i18ncmp expect output
 '
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index b527f8009c..74553f991b 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -33,7 +33,7 @@ fill () {
 
 
 test_expect_success setup '
-	fill x y z > same &&
+	fill x y z >same &&
 	fill 1 2 3 4 5 6 7 8 >one &&
 	fill a b c d e >two &&
 	git add same one two &&
@@ -55,7 +55,7 @@ test_expect_success setup '
 
 	git checkout -b simple master &&
 	rm -f one &&
-	fill a c e > two &&
+	fill a c e >two &&
 	git commit -a -m "Simple D one, M two" &&
 
 	git checkout master
@@ -95,7 +95,7 @@ test_expect_success 'checkout -m with dirty tree' '
 	git clean -f &&
 
 	fill 0 1 2 3 4 5 6 7 8 >one &&
-	git checkout -m side > messages &&
+	git checkout -m side >messages &&
 
 	test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
 
@@ -159,7 +159,7 @@ test_expect_success 'checkout -m with merge conflict' '
 test_expect_success 'format of merge conflict from checkout -m' '
 	git checkout -f master && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout -m simple &&
 
 	git ls-files >current &&
@@ -182,7 +182,7 @@ test_expect_success 'format of merge conflict from checkout -m' '
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 	git checkout -f master && git reset --hard && git clean -f &&
 
-	fill b d > two &&
+	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
 
 	cat <<-EOF >expect &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 4/5][Outreachy] t7201: use 'git -C' to avoid subshell
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                         ` (8 preceding siblings ...)
  2020-10-22  7:16       ` [PATCH v6 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
@ 2020-10-22  7:16       ` Charvi Mendiratta
  2020-10-22  7:16       ` [PATCH v6 5/5][Outreachy] t7201: put each command on a separate line Charvi Mendiratta
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-22  7:16 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, me, phillip.wood123, sunshine,
	Charvi Mendiratta

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 74553f991b..5898182fd2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -339,10 +339,7 @@ test_expect_success 'switch branches while in subdirectory' '
 	git checkout master &&
 
 	mkdir subs &&
-	(
-		cd subs &&
-		git checkout side
-	) &&
+	git -C subs checkout side &&
 	! test -f subs/one &&
 	rm -fr subs
 '
@@ -357,10 +354,7 @@ test_expect_success 'checkout specific path while in subdirectory' '
 
 	git checkout master &&
 	mkdir -p subs &&
-	(
-		cd subs &&
-		git checkout side -- bero
-	) &&
+	git -C subs checkout side -- bero &&
 	test -f subs/bero
 '
 
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 5/5][Outreachy] t7201: put each command on a separate line
  2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
                         ` (9 preceding siblings ...)
  2020-10-22  7:16       ` [PATCH v6 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
@ 2020-10-22  7:16       ` Charvi Mendiratta
  10 siblings, 0 replies; 60+ messages in thread
From: Charvi Mendiratta @ 2020-10-22  7:16 UTC (permalink / raw)
  To: git
  Cc: gitster, christian.couder, me, phillip.wood123, sunshine,
	Charvi Mendiratta

Modern practice is to avoid multiple commands per line,
and instead place each command on its own line.

Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7201-co.sh | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5898182fd2..b36a93056f 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -157,7 +157,8 @@ test_expect_success 'checkout -m with merge conflict' '
 '
 
 test_expect_success 'format of merge conflict from checkout -m' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout -m simple &&
@@ -180,7 +181,9 @@ test_expect_success 'format of merge conflict from checkout -m' '
 '
 
 test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 
 	fill b d >two &&
 	git checkout --merge --conflict=diff3 simple &&
@@ -205,7 +208,9 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 '
 
 test_expect_success 'switch to another branch while carrying a deletion' '
-	git checkout -f master && git reset --hard && git clean -f &&
+	git checkout -f master &&
+	git reset --hard &&
+	git clean -f &&
 	git rm two &&
 
 	test_must_fail git checkout simple 2>errs &&
@@ -218,7 +223,8 @@ test_expect_success 'switch to another branch while carrying a deletion' '
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 	git config advice.detachedHead false &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	git checkout renamer^ 2>messages &&
 	test_i18ngrep "HEAD is now at $rev" messages &&
 	test_line_count = 1 messages &&
@@ -237,7 +243,8 @@ test_expect_success 'checkout to detach HEAD (with advice declined)' '
 test_expect_success 'checkout to detach HEAD' '
 	git config advice.detachedHead true &&
 	rev=$(git rev-parse --short renamer^) &&
-	git checkout -f renamer && git clean -f &&
+	git checkout -f renamer &&
+	git clean -f &&
 	GIT_TEST_GETTEXT_POISON=false git checkout renamer^ 2>messages &&
 	grep "HEAD is now at $rev" messages &&
 	test_line_count -gt 1 messages &&
@@ -254,7 +261,8 @@ test_expect_success 'checkout to detach HEAD' '
 '
 
 test_expect_success 'checkout to detach HEAD with branchname^' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout renamer^ &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -269,7 +277,8 @@ test_expect_success 'checkout to detach HEAD with branchname^' '
 '
 
 test_expect_success 'checkout to detach HEAD with :/message' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout ":/Initial" &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
@@ -284,7 +293,8 @@ test_expect_success 'checkout to detach HEAD with :/message' '
 '
 
 test_expect_success 'checkout to detach HEAD with HEAD^0' '
-	git checkout -f master && git clean -f &&
+	git checkout -f master &&
+	git clean -f &&
 	git checkout HEAD^0 &&
 	H=$(git rev-parse --verify HEAD) &&
 	M=$(git show-ref -s --verify refs/heads/master) &&
-- 
2.29.0.rc1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator
  2020-10-22  6:04               ` Eric Sunshine
@ 2020-10-22 17:35                 ` Junio C Hamano
  0 siblings, 0 replies; 60+ messages in thread
From: Junio C Hamano @ 2020-10-22 17:35 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Charvi Mendiratta, Git List, Christian Couder, Taylor Blau, Phillip Wood

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Thu, Oct 22, 2020 at 1:56 AM Junio C Hamano <gitster@pobox.com> wrote:
>> Subject: t7102: prepare expected output inside test_expect_* block
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> diff --git c/t/t7102-reset.sh w/t/t7102-reset.sh
>> @@ -82,15 +82,15 @@ test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' '
>> ->.diff_expect
>> ->.cached_expect
>> -cat >.cat_expect <<EOF
>> -secondfile:
>> -EOF
>> -
>>  test_expect_success 'giving a non existing revision should fail' '
>> +       >.diff_expect &&
>> +       >.cached_expect &&
>> +       cat >.cat_expect <<-\EOF &&
>> +       secondfile:
>> +       EOF
>
> You used <<-\EOF rather than plain <<-EOF when possible. Good.
>
> (Might be worth mention in the commit message, but perhaps too minor?)
>
>> @@ -191,38 +191,38 @@ test_expect_success 'resetting to HEAD with no changes should succeed and do not
>>  test_expect_success '--soft reset only should show changes in diff --cached' '
>> +       >.diff_expect &&
>> +       cat >.cached_expect <<-EOF &&
>> +       diff --git a/secondfile b/secondfile
>> +       index $head5p1s..$head5s 100644
>
> And used plain <<-EOF when necessary. Fine.

Yup.

Let's declare victory with v5 plus this one as [patch 6/5], and move
on.

We both know very well that through the microproject that it has
been demonstrated that Charvi can now comfortably work with us in
the review discussions.

Thanks, both.

^ permalink raw reply	[flat|nested] 60+ messages in thread

end of thread, other threads:[~2020-10-22 17:35 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15 17:57 [PATCH 0/5][Outreachy] modernizing the test scripts charvi-077
2020-10-15 17:57 ` [PATCH 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting charvi-077
2020-10-16 13:07   ` Christian Couder
2020-10-15 17:57 ` [PATCH 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body charvi-077
2020-10-15 17:57 ` [PATCH 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator charvi-077
2020-10-15 17:57 ` [PATCH 4/5][Outreachy] t7201: avoid using cd outside of subshells charvi-077
2020-10-15 17:57 ` [PATCH 5/5][Outreachy] t7201: place each command in its own line charvi-077
2020-10-16 12:54 ` [PATCH 0/5][Outreachy] modernizing the test scripts Christian Couder
2020-10-17  8:27   ` Charvi Mendiratta
2020-10-17  7:54 ` [PATCH v2 " Charvi Mendiratta
2020-10-17  7:54   ` [PATCH v2 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
2020-10-17  7:54   ` [PATCH v2 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
2020-10-17 15:13     ` Đoàn Trần Công Danh
2020-10-18  5:40       ` Charvi Mendiratta
2020-10-17  7:54   ` [PATCH v2 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
2020-10-17  7:54   ` [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells Charvi Mendiratta
2020-10-18 15:39     ` Phillip Wood
2020-10-19 12:55       ` Charvi Mendiratta
2020-10-19 13:46         ` Phillip Wood
2020-10-19 17:24           ` Charvi Mendiratta
2020-10-19 20:25             ` Taylor Blau
2020-10-20  5:38               ` Charvi Mendiratta
2020-10-20 20:09                 ` Taylor Blau
2020-10-20  9:13               ` Phillip Wood
2020-10-20 11:48                 ` Charvi Mendiratta
2020-10-17  7:54   ` [PATCH v2 5/5][Outreachy] t7201: place each command in its own line Charvi Mendiratta
2020-10-20 11:43   ` [PATCH v3 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
2020-10-20 11:43     ` [PATCH v3 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
2020-10-20 11:43     ` [PATCH v3 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
2020-10-20 11:43     ` [PATCH v3 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
2020-10-20 11:43     ` [PATCH v3 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
2020-10-20 11:43     ` [PATCH v3 5/5][Outreachy] t7201: put each command on a seperate line Charvi Mendiratta
2020-10-20 12:11   ` [PATCH v4] t7201: put each command on a separate line Charvi Mendiratta
2020-10-20 20:13     ` Junio C Hamano
2020-10-20 20:15       ` Taylor Blau
2020-10-20 20:25         ` Junio C Hamano
2020-10-20 20:30           ` Taylor Blau
2020-10-20 21:00             ` Junio C Hamano
2020-10-21  7:14             ` Charvi Mendiratta
2020-10-20 20:19       ` Junio C Hamano
2020-10-21 13:16         ` Charvi Mendiratta
2020-10-21 12:48     ` [PATCH v5 0/5][Outreachy] modernize the test scripts Charvi Mendiratta
2020-10-21 12:48       ` [PATCH v5 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
2020-10-21 12:48       ` [PATCH v5 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
2020-10-21 12:48       ` [PATCH v5 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
2020-10-21 17:20         ` Eric Sunshine
2020-10-22  5:44           ` Junio C Hamano
2020-10-22  5:53             ` Eric Sunshine
2020-10-22  5:55             ` Junio C Hamano
2020-10-22  6:04               ` Eric Sunshine
2020-10-22 17:35                 ` Junio C Hamano
2020-10-22  6:29             ` Charvi Mendiratta
2020-10-21 12:48       ` [PATCH v5 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
2020-10-21 12:48       ` [PATCH v5 5/5][Outreachy] t7201: put each command on a separate line Charvi Mendiratta
2020-10-22  7:16       ` [PATCH v6 0/5][Outreachy] modernize test scripts Charvi Mendiratta
2020-10-22  7:16       ` [PATCH v6 1/5][Outreachy] t7101,t7102,t7201: modernize test formatting Charvi Mendiratta
2020-10-22  7:16       ` [PATCH v6 2/5][Outreachy] t7102,t7201: remove unnecessary blank spaces in test body Charvi Mendiratta
2020-10-22  7:16       ` [PATCH v6 3/5][Outreachy] t7102,t7201: remove whitespace after redirect operator Charvi Mendiratta
2020-10-22  7:16       ` [PATCH v6 4/5][Outreachy] t7201: use 'git -C' to avoid subshell Charvi Mendiratta
2020-10-22  7:16       ` [PATCH v6 5/5][Outreachy] t7201: put each command on a separate line Charvi Mendiratta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).