diff mbox series

[03/11] t7001: remove unnecessary blank lines

Message ID 20200925170256.11490-4-shubhunic@gmail.com
State New
Headers show
Series Modernizing the t7001 test script | expand

Commit Message

Shubham Verma Sept. 25, 2020, 5:02 p.m. UTC
From: Shubham Verma <shubhunic@gmail.com>

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: shubham verma <shubhunic@gmail.com>
---
 t/t7001-mv.sh | 12 ------------
 1 file changed, 12 deletions(-)

Comments

Eric Sunshine Sept. 25, 2020, 5:50 p.m. UTC | #1
On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:
> 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.

s/So we should removed/Remove/

Otherwise, nice explanatory commit message.

One comment below not related directly to this patch (just something
noticed in the patch conext lines)...

> Signed-off-by: shubham verma <shubhunic@gmail.com>
> ---
> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> @@ -182,7 +182,6 @@ test_expect_success "Sergey Vlasov's test case" '
>  test_expect_success 'absolute pathname' '(
>      ...
>  )'
>  test_expect_success 'absolute pathname outside should fail' '(
>      ...
>  )'

It is very uncommon style to hide the subshell as these two tests do:

    test_expect_success 'title' '(
        ...
    )'

Instead, these should be formatted as:

    test_expect_success 'title' '
        (
            ...
        )
    '

Note that the "(" and ")" of the subshell are indented with a TAB, and
then the body of the subshell is indented again with another TAB in
order to comply with current style guidelines.

Fixing these might possibly be done in patch [1/11], however, they are
so unusual and would change indentation of the body lines that they
might deserve a patch of their own to avoid being lost in the noise of
[1/11].
Junio C Hamano Sept. 25, 2020, 8:19 p.m. UTC | #2
Eric Sunshine <sunshine@sunshineco.com> writes:

> It is very uncommon style to hide the subshell as these two tests do:
>
>     test_expect_success 'title' '(
>         ...
>     )'
>
> Instead, these should be formatted as:
>
>     test_expect_success 'title' '
>         (
>             ...
>         )
>     '
>
> Note that the "(" and ")" of the subshell are indented with a TAB, and
> then the body of the subshell is indented again with another TAB in
> order to comply with current style guidelines.
>
> Fixing these might possibly be done in patch [1/11], however, they are
> so unusual and would change indentation of the body lines that they
> might deserve a patch of their own to avoid being lost in the noise of
> [1/11].

I agree that adding that to 01/11 might be too noisy, but 04/11 may
be a good match.

Thanks.
diff mbox series

Patch

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 7503233814..f63802442b 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -182,7 +182,6 @@  test_expect_success "Sergey Vlasov's test case" '
 '
 
 test_expect_success 'absolute pathname' '(
-
 	rm -fr mine &&
 	mkdir mine &&
 	cd mine &&
@@ -196,12 +195,9 @@  test_expect_success 'absolute pathname' '(
 	! test -d sub &&
 	test -d in &&
 	git ls-files --error-unmatch in/file
-
-
 )'
 
 test_expect_success 'absolute pathname outside should fail' '(
-
 	rm -fr mine &&
 	mkdir mine &&
 	cd mine &&
@@ -216,7 +212,6 @@  test_expect_success 'absolute pathname outside should fail' '(
 	test -d sub &&
 	! test -d ../in &&
 	git ls-files --error-unmatch sub/file
-
 )'
 
 test_expect_success 'git mv to move multiple sources into a directory' '
@@ -232,7 +227,6 @@  test_expect_success 'git mv to move multiple sources into a directory' '
 '
 
 test_expect_success 'git mv should not change sha1 of moved cache entry' '
-
 	rm -fr .git &&
 	git init &&
 	echo 1 >dirty &&
@@ -243,7 +237,6 @@  test_expect_success 'git mv should not change sha1 of moved cache entry' '
 	echo 2 >dirty2 &&
 	git mv dirty2 dirty &&
 	[ "$entry" = "$(git ls-files --stage dirty | cut -f 1)" ]
-
 '
 
 rm -f dirty dirty2
@@ -266,7 +259,6 @@  test_expect_success 'git mv error on conflicted file' '
 '
 
 test_expect_success 'git mv should overwrite symlink to a file' '
-
 	rm -fr .git &&
 	git init &&
 	echo 1 >moved &&
@@ -279,13 +271,11 @@  test_expect_success 'git mv should overwrite symlink to a file' '
 	test "$(cat symlink)" = 1 &&
 	git update-index --refresh &&
 	git diff-files --quiet
-
 '
 
 rm -f moved symlink
 
 test_expect_success 'git mv should overwrite file with a symlink' '
-
 	rm -fr .git &&
 	git init &&
 	echo 1 >moved &&
@@ -296,11 +286,9 @@  test_expect_success 'git mv should overwrite file with a symlink' '
 	! test -e symlink &&
 	git update-index --refresh &&
 	git diff-files --quiet
-
 '
 
 test_expect_success SYMLINKS 'check moved symlink' '
-
 	test -h moved
 '