[GSOC] [PATCH v1 0/2] Modernize t9700 test script

[GSOC] [PATCH v1 0/2] Modernize t9700 test script

[Zhang Yi]

A title in the test script is in old style an a body is indented by spaces.I fix the title style and change indents to TAB.

Zhang Yi (2):
  Fix title style
  Change indents  to TAB

 t/t9700-perl-git.sh | 61 ++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

-- 
2.29.0-rc0

[GSOC] [PATCH v1 1/2] Fix title style

[Zhang Yi]

Signed-off-by: Zhang Yi <18994118902@163.com>
---
 t/t9700-perl-git.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index b105d6d9d5..3f6396ef63 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -13,9 +13,8 @@ skip_all_if_no_Test_More
 
 # set up test repository
 
-test_expect_success \
-    'set up test repository' \
-    'echo "test file 1" > file1 &&
+test_expect_success 'set up test repository' '
+     echo "test file 1" > file1 &&
      echo "test file 2" > file2 &&
      mkdir directory1 &&
      echo "in directory1" >> directory1/file &&
-- 
2.29.0-rc0

[GSOC] [PATCH v1 2/2] Change indents to TAB

[Zhang Yi]

Signed-off-by: Zhang Yi <18994118902@163.com>
---
 t/t9700-perl-git.sh | 58 ++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 3f6396ef63..671875a3fc 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -14,35 +14,35 @@ skip_all_if_no_Test_More
 # set up test repository
 
 test_expect_success 'set up test repository' '
-     echo "test file 1" > file1 &&
-     echo "test file 2" > file2 &&
-     mkdir directory1 &&
-     echo "in directory1" >> directory1/file &&
-     mkdir directory2 &&
-     echo "in directory2" >> directory2/file &&
-     git add . &&
-     git commit -m "first commit" &&
-
-     echo "new file in subdir 2" > directory2/file2 &&
-     git add . &&
-     git commit -m "commit in directory2" &&
-
-     echo "changed file 1" > file1 &&
-     git commit -a -m "second commit" &&
-
-     git config --add color.test.slot1 green &&
-     git config --add test.string value &&
-     git config --add test.dupstring value1 &&
-     git config --add test.dupstring value2 &&
-     git config --add test.booltrue true &&
-     git config --add test.boolfalse no &&
-     git config --add test.boolother other &&
-     git config --add test.int 2k &&
-     git config --add test.path "~/foo" &&
-     git config --add test.pathexpanded "$HOME/foo" &&
-     git config --add test.pathmulti foo &&
-     git config --add test.pathmulti bar
-     '
+	echo "test file 1" > file1 &&
+	echo "test file 2" > file2 &&
+	mkdir directory1 &&
+	echo "in directory1" >> directory1/file &&
+	mkdir directory2 &&
+	echo "in directory2" >> directory2/file &&
+	git add . &&
+	git commit -m "first commit" &&
+
+	echo "new file in subdir 2" > directory2/file2 &&
+	git add . &&
+	git commit -m "commit in directory2" &&
+
+	echo "changed file 1" > file1 &&
+	git commit -a -m "second commit" &&
+
+	git config --add color.test.slot1 green &&
+	git config --add test.string value &&
+	git config --add test.dupstring value1 &&
+	git config --add test.dupstring value2 &&
+	git config --add test.booltrue true &&
+	git config --add test.boolfalse no &&
+	git config --add test.boolother other &&
+	git config --add test.int 2k &&
+	git config --add test.path "~/foo" &&
+	git config --add test.pathexpanded "$HOME/foo" &&
+	git config --add test.pathmulti foo &&
+	git config --add test.pathmulti bar
+'
 
 test_expect_success 'set up bare repository' '
 	git init --bare bare.git
-- 
2.29.0-rc0

Re: [GSOC] [PATCH v1 0/2] Modernize t9700 test script

[Vivan Garg]

Hi Zhang Yi,

> Subject: [GSOC] [PATCH v1 0/2] Modernize t9700 test script

It is the convention here to start the subject line with the name of the
component you're working on, and it's always good to follow the
convention. Something like:

t9700: modernize test style

Note that the part after the colon should start with a lowercase letter.

> A title in the test script is in old style an a body is indented by spaces.I fix the title style and change indents to TAB.
>
> Zhang Yi (2):
>   Fix title style
>   Change indents  to TAB
>
>  t/t9700-perl-git.sh | 61 ++++++++++++++++++++++-----------------------
>  1 file changed, 30 insertions(+), 31 deletions(-)
>
> --
> 2.29.0-rc0
>

Re: [GSOC] [PATCH v1 1/2] Fix title style

[Vivan Garg]

You are missing the body of the commit message. Commit message
should start with description of the existing problem in present tense,
something like:

     Test scripts in file t9700-perl-git.sh are written in old style,
     where the test_expect_success command and test title are written on
     separate lines ...

Then changes should be described using imperative mood, as if you are
giving commands to the codebase.  See section "[[describe-changes]]"
in "Documentation/SubmittingPatches" for details.

> Signed-off-by: Zhang Yi <18994118902@163.com>
> ---
>  t/t9700-perl-git.sh | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
> index b105d6d9d5..3f6396ef63 100755
> --- a/t/t9700-perl-git.sh
> +++ b/t/t9700-perl-git.sh
> @@ -13,9 +13,8 @@ skip_all_if_no_Test_More
>
>  # set up test repository
>
> -test_expect_success \
> -    'set up test repository' \
> -    'echo "test file 1" > file1 &&
> +test_expect_success 'set up test repository' '
> +     echo "test file 1" > file1 &&
>       echo "test file 2" > file2 &&
>       mkdir directory1 &&
>       echo "in directory1" >> directory1/file &&
> --

there should be no whitespace after redirect operators, so:

    echo "test file 1" > file1 &&

  should become:

    echo "test file 1" >file1 &&

Similar changes for the rest of the test script as well.

> 2.29.0-rc0
>

Re: [GSOC] [PATCH v1 2/2] Change indents to TAB

[Vivan Garg]

You are missing the body of the commit message here as well, Please add one
in your next re-roll.

On Tue, Feb 21, 2023 at 10:01 PM Zhang Yi <18994118902@163.com> wrote:
>
> Signed-off-by: Zhang Yi <18994118902@163.com>
> ---
>  t/t9700-perl-git.sh | 58 ++++++++++++++++++++++-----------------------
>  1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
> index 3f6396ef63..671875a3fc 100755
> --- a/t/t9700-perl-git.sh
> +++ b/t/t9700-perl-git.sh
> @@ -14,35 +14,35 @@ skip_all_if_no_Test_More
>  # set up test repository
>
>  test_expect_success 'set up test repository' '
> -     echo "test file 1" > file1 &&
> -     echo "test file 2" > file2 &&
> -     mkdir directory1 &&
> -     echo "in directory1" >> directory1/file &&
> -     mkdir directory2 &&
> -     echo "in directory2" >> directory2/file &&
> -     git add . &&
> -     git commit -m "first commit" &&
> -
> -     echo "new file in subdir 2" > directory2/file2 &&
> -     git add . &&
> -     git commit -m "commit in directory2" &&
> -
> -     echo "changed file 1" > file1 &&
> -     git commit -a -m "second commit" &&
> -
> -     git config --add color.test.slot1 green &&
> -     git config --add test.string value &&
> -     git config --add test.dupstring value1 &&
> -     git config --add test.dupstring value2 &&
> -     git config --add test.booltrue true &&
> -     git config --add test.boolfalse no &&
> -     git config --add test.boolother other &&
> -     git config --add test.int 2k &&
> -     git config --add test.path "~/foo" &&
> -     git config --add test.pathexpanded "$HOME/foo" &&
> -     git config --add test.pathmulti foo &&
> -     git config --add test.pathmulti bar
> -     '
> +       echo "test file 1" > file1 &&
> +       echo "test file 2" > file2 &&
> +       mkdir directory1 &&
> +       echo "in directory1" >> directory1/file &&
> +       mkdir directory2 &&
> +       echo "in directory2" >> directory2/file &&
> +       git add . &&
> +       git commit -m "first commit" &&
> +
> +       echo "new file in subdir 2" > directory2/file2 &&
> +       git add . &&
> +       git commit -m "commit in directory2" &&
> +
> +       echo "changed file 1" > file1 &&
> +       git commit -a -m "second commit" &&
> +
> +       git config --add color.test.slot1 green &&
> +       git config --add test.string value &&
> +       git config --add test.dupstring value1 &&
> +       git config --add test.dupstring value2 &&
> +       git config --add test.booltrue true &&
> +       git config --add test.boolfalse no &&
> +       git config --add test.boolother other &&
> +       git config --add test.int 2k &&
> +       git config --add test.path "~/foo" &&
> +       git config --add test.pathexpanded "$HOME/foo" &&
> +       git config --add test.pathmulti foo &&
> +       git config --add test.pathmulti bar
> +'
>
>  test_expect_success 'set up bare repository' '
>         git init --bare bare.git
> --

There should be no whitespace after redirect operators; as described in the last
commit of this patch. Make sure to fix that in your next re-roll for
this test script
as well. You might also want to look at this for a summary of all the
things you
need to do:
https://lore.kernel.org/git/CAPig+cQpUu2UO-+jWn1nTaDykWnxwuEitzVB7PnW2SS_b7V8Hg@mail.gmail.com/

> 2.29.0-rc0
>

Thanks!

[GSOC] [PATCH v2 0/3] t9700:moderizen test scripts

[Zhang Yi]

The style of t9700-perl-git.sh is old.There are 3 things need improve:
* A title is not on the same line with test_expect_success command.Need to combine them.
* A test body is indented by whitespaces.Need to replace whitespaces by TAB
* There are whitespaces after redirect operators.Need to delete whitespaces.

---
Thanks for all suggestions.Here are so many interesting things to learn.

Zhang Yi (3):
  t9700:fix title style
  t9700:change indents to TAB
  t9700:delete whitespaces after redirect operators

 t/t9700-perl-git.sh | 61 ++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

-- 
2.29.0-rc0

[GSOC] [PATCH v2 1/3] t9700:fix title style

[Zhang Yi]

One title in t9700-perl-git.sh is in old style where the
test_expect_success command and test title are written on
separate lines.

Combine the test_expect_success command and the title in one line.

Signed-off-by: Zhang Yi <18994118902@163.com>
---
 t/t9700-perl-git.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index b105d6d9d5..3f6396ef63 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -13,9 +13,8 @@ skip_all_if_no_Test_More
 
 # set up test repository
 
-test_expect_success \
-    'set up test repository' \
-    'echo "test file 1" > file1 &&
+test_expect_success 'set up test repository' '
+     echo "test file 1" > file1 &&
      echo "test file 2" > file2 &&
      mkdir directory1 &&
      echo "in directory1" >> directory1/file &&
-- 
2.29.0-rc0

[GSOC] [PATCH v2 2/3] t9700:change indents to TAB

[Zhang Yi]

The body of one test script in t9700-perl-git.sh is indented by spaces
while others are indented by TAB.And test bodies should be indented with
TAB, not spaces.

Replace all space indents by TAB.

Signed-off-by: Zhang Yi <18994118902@163.com>
---
 t/t9700-perl-git.sh | 58 ++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 3f6396ef63..671875a3fc 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -14,35 +14,35 @@ skip_all_if_no_Test_More
 # set up test repository
 
 test_expect_success 'set up test repository' '
-     echo "test file 1" > file1 &&
-     echo "test file 2" > file2 &&
-     mkdir directory1 &&
-     echo "in directory1" >> directory1/file &&
-     mkdir directory2 &&
-     echo "in directory2" >> directory2/file &&
-     git add . &&
-     git commit -m "first commit" &&
-
-     echo "new file in subdir 2" > directory2/file2 &&
-     git add . &&
-     git commit -m "commit in directory2" &&
-
-     echo "changed file 1" > file1 &&
-     git commit -a -m "second commit" &&
-
-     git config --add color.test.slot1 green &&
-     git config --add test.string value &&
-     git config --add test.dupstring value1 &&
-     git config --add test.dupstring value2 &&
-     git config --add test.booltrue true &&
-     git config --add test.boolfalse no &&
-     git config --add test.boolother other &&
-     git config --add test.int 2k &&
-     git config --add test.path "~/foo" &&
-     git config --add test.pathexpanded "$HOME/foo" &&
-     git config --add test.pathmulti foo &&
-     git config --add test.pathmulti bar
-     '
+	echo "test file 1" > file1 &&
+	echo "test file 2" > file2 &&
+	mkdir directory1 &&
+	echo "in directory1" >> directory1/file &&
+	mkdir directory2 &&
+	echo "in directory2" >> directory2/file &&
+	git add . &&
+	git commit -m "first commit" &&
+
+	echo "new file in subdir 2" > directory2/file2 &&
+	git add . &&
+	git commit -m "commit in directory2" &&
+
+	echo "changed file 1" > file1 &&
+	git commit -a -m "second commit" &&
+
+	git config --add color.test.slot1 green &&
+	git config --add test.string value &&
+	git config --add test.dupstring value1 &&
+	git config --add test.dupstring value2 &&
+	git config --add test.booltrue true &&
+	git config --add test.boolfalse no &&
+	git config --add test.boolother other &&
+	git config --add test.int 2k &&
+	git config --add test.path "~/foo" &&
+	git config --add test.pathexpanded "$HOME/foo" &&
+	git config --add test.pathmulti foo &&
+	git config --add test.pathmulti bar
+'
 
 test_expect_success 'set up bare repository' '
 	git init --bare bare.git
-- 
2.29.0-rc0

[GSOC] [PATCH v2 3/3] t9700:delete whitespaces after redirect operators

[Zhang Yi]

There are whitespaces after redirect operators,which is a deprecated
style.

Delete whitespaces after redirect operators.

Signed-off-by: Zhang Yi <18994118902@163.com>
---
 t/t9700-perl-git.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 671875a3fc..ccc8212d73 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -14,20 +14,20 @@ skip_all_if_no_Test_More
 # set up test repository
 
 test_expect_success 'set up test repository' '
-	echo "test file 1" > file1 &&
-	echo "test file 2" > file2 &&
+	echo "test file 1" >file1 &&
+	echo "test file 2" >file2 &&
 	mkdir directory1 &&
-	echo "in directory1" >> directory1/file &&
+	echo "in directory1" >>directory1/file &&
 	mkdir directory2 &&
-	echo "in directory2" >> directory2/file &&
+	echo "in directory2" >>directory2/file &&
 	git add . &&
 	git commit -m "first commit" &&
 
-	echo "new file in subdir 2" > directory2/file2 &&
+	echo "new file in subdir 2" >directory2/file2 &&
 	git add . &&
 	git commit -m "commit in directory2" &&
 
-	echo "changed file 1" > file1 &&
+	echo "changed file 1" >file1 &&
 	git commit -a -m "second commit" &&
 
 	git config --add color.test.slot1 green &&
-- 
2.29.0-rc0

Re: [GSOC] [PATCH v2 0/3] t9700:moderizen test scripts

[Junio C Hamano]

Zhang Yi <18994118902@163.com> writes:

> The style of t9700-perl-git.sh is old.There are 3 things need improve:
> * A title is not on the same line with test_expect_success command.Need to combine them.
> * A test body is indented by whitespaces.Need to replace whitespaces by TAB
> * There are whitespaces after redirect operators.Need to delete whitespaces.
>
> ---
> Thanks for all suggestions.Here are so many interesting things to learn.

It is a good idea to summarize the difference between the previous
round and this round, to help reviewers who have already spent
cycles to review the previous round to skim the parts of the updated
series that haven't been changed (and they have already reviewed).

And the cover letter is the ideal place to do so.

Alternatively, you can write after the three-dash lines between the
proposed log message and the patch text of each patch message.

> Zhang Yi (3):
>   t9700:fix title style
>   t9700:change indents to TAB
>   t9700:delete whitespaces after redirect operators

You are missing space after the colon, all of them.  Also you are
missing space after full-stop that concludes each sentence in all of
your proposed log messages.

Even though you _could_ treat these three things as logically
separate changes, I would suspect that each of them is so small that
it may be easier to review as a single patch.

Thanks.

Re:Re: [GSOC] [PATCH v2 0/3] t9700:moderizen test scripts

[Zhang Yi]

Ok. Thanks.

So I need to do:

* Summary the update between versions in the cover letter.
* Add space after colon in patch titles.
* Add space after full-stop in proposed log messages. 
Just like "The style of t9700-perl-git.sh is old. There are 3 things need improve".
* Combine 3 patches into 1.

Hope no misunderstanding. If something wrong, please address me.

Re: [GSOC] [PATCH v2 3/3] t9700:delete whitespaces after redirect operators

[Philip Oakley]

On 24/02/2023 01:34, Zhang Yi wrote:
> There are whitespaces after redirect operators,which is a deprecated

micro-nit: space after comma, before "which".

Philip
> style.
>
> Delete whitespaces after redirect operators.
>
> Signed-off-by: Zhang Yi <18994118902@163.com>
> ---
>  t/t9700-perl-git.sh | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
> index 671875a3fc..ccc8212d73 100755
> --- a/t/t9700-perl-git.sh
> +++ b/t/t9700-perl-git.sh
> @@ -14,20 +14,20 @@ skip_all_if_no_Test_More
>  # set up test repository
>  
>  test_expect_success 'set up test repository' '
> -	echo "test file 1" > file1 &&
> -	echo "test file 2" > file2 &&
> +	echo "test file 1" >file1 &&
> +	echo "test file 2" >file2 &&
>  	mkdir directory1 &&
> -	echo "in directory1" >> directory1/file &&
> +	echo "in directory1" >>directory1/file &&
>  	mkdir directory2 &&
> -	echo "in directory2" >> directory2/file &&
> +	echo "in directory2" >>directory2/file &&
>  	git add . &&
>  	git commit -m "first commit" &&
>  
> -	echo "new file in subdir 2" > directory2/file2 &&
> +	echo "new file in subdir 2" >directory2/file2 &&
>  	git add . &&
>  	git commit -m "commit in directory2" &&
>  
> -	echo "changed file 1" > file1 &&
> +	echo "changed file 1" >file1 &&
>  	git commit -a -m "second commit" &&
>  
>  	git config --add color.test.slot1 green &&

[GSOC] [PATCH v3 1/1] t9700: modernize test scripts

[Zhang Yi]

The style of t9700-perl-git.sh is old. There are 3 problems:
* A title is not on the same line with test_expect_success command.
* A test body is indented by whitespaces.
* There are whitespaces after redirect operators.

Modernize test scripts by:
* Combine the title with test_expect_success command.
* Replace whitespace indents with TAB.
* Delete whitespaces after redirect operators.

Signed-off-by: Zhang Yi <18994118902@163.com>
---
Update compare to v2:
* Add summary between versions.
* Add space after colon in patch title.
* Add space after full stops and after comma.
* Combine 3 patches into 1.

After review 'First steps contributing to Git', I find it available to
send a single update patch for new version. I think this way is eaiser to
review. 

Thanks for help from Junio C Hamano, Philip Oaklcy and again for Vivan
Garg. 

 t/t9700-perl-git.sh | 61 ++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 31 deletions(-)


diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index b105d6d9d5..ccc8212d73 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -13,37 +13,36 @@ skip_all_if_no_Test_More
 
 # set up test repository
 
-test_expect_success \
-    'set up test repository' \
-    'echo "test file 1" > file1 &&
-     echo "test file 2" > file2 &&
-     mkdir directory1 &&
-     echo "in directory1" >> directory1/file &&
-     mkdir directory2 &&
-     echo "in directory2" >> directory2/file &&
-     git add . &&
-     git commit -m "first commit" &&
-
-     echo "new file in subdir 2" > directory2/file2 &&
-     git add . &&
-     git commit -m "commit in directory2" &&
-
-     echo "changed file 1" > file1 &&
-     git commit -a -m "second commit" &&
-
-     git config --add color.test.slot1 green &&
-     git config --add test.string value &&
-     git config --add test.dupstring value1 &&
-     git config --add test.dupstring value2 &&
-     git config --add test.booltrue true &&
-     git config --add test.boolfalse no &&
-     git config --add test.boolother other &&
-     git config --add test.int 2k &&
-     git config --add test.path "~/foo" &&
-     git config --add test.pathexpanded "$HOME/foo" &&
-     git config --add test.pathmulti foo &&
-     git config --add test.pathmulti bar
-     '
+test_expect_success 'set up test repository' '
+	echo "test file 1" >file1 &&
+	echo "test file 2" >file2 &&
+	mkdir directory1 &&
+	echo "in directory1" >>directory1/file &&
+	mkdir directory2 &&
+	echo "in directory2" >>directory2/file &&
+	git add . &&
+	git commit -m "first commit" &&
+
+	echo "new file in subdir 2" >directory2/file2 &&
+	git add . &&
+	git commit -m "commit in directory2" &&
+
+	echo "changed file 1" >file1 &&
+	git commit -a -m "second commit" &&
+
+	git config --add color.test.slot1 green &&
+	git config --add test.string value &&
+	git config --add test.dupstring value1 &&
+	git config --add test.dupstring value2 &&
+	git config --add test.booltrue true &&
+	git config --add test.boolfalse no &&
+	git config --add test.boolother other &&
+	git config --add test.int 2k &&
+	git config --add test.path "~/foo" &&
+	git config --add test.pathexpanded "$HOME/foo" &&
+	git config --add test.pathmulti foo &&
+	git config --add test.pathmulti bar
+'
 
 test_expect_success 'set up bare repository' '
 	git init --bare bare.git
-- 
2.29.0-rc0