| Message ID | 20200925170256.11490-4-shubhunic@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Modernizing the t7001 test script | expand |
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].
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 --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 '