Web lists-archives.com

Re: [GSoC][PATCH v3 2/3] t3600: modernize style




On Mon, Mar 4, 2019 at 7:08 AM Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx> wrote:
> The tests in `t3600-rm.sh` were written  long time ago, and has a lot of
> style violations, including the mixed use of tabs and spaces, not having
> the title  and the  opening quote of the body on  the first line of  the
> tests, and other  shell script  style violations. Update it to match the
> CodingGuidelines.

Many of the words in this commit message are separated by multiple
spaces. Please fold out the excess so there is only a single space
between words.

> Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx>
> ---
> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
> @@ -217,23 +218,25 @@ test_expect_success 'Remove nonexistent file returns nonzero exit status' '
>  test_expect_success 'Call "rm" from outside the work tree' '
>         mkdir repo &&
> +       (
> +               cd repo &&
> +               git init &&
> +               echo something >somefile &&
> +               git add somefile &&
> +               git commit -m "add a file" &&
> +               (
> +                       cd .. &&
> +                       git --git-dir=repo/.git --work-tree=repo rm somefile
> +               ) &&
> +               test_must_fail git ls-files --error-unmatch somefile
> +       )
>  '

This test is unusual in that it first cd's into a subdirectory and
then cd's back out with "cd ..". And, while the use of subshells is
correct to ensure that all 'cd' commands are undone at the end of the
test (whether successful or not), the entire construction is
unnecessarily confusing. This is not the sort of issue which should be
fixed in this style-fix patch, however, it is something which could be
cleaned up with a follow-up patch. For instance, the test might be
reworked like this:

    git init repo &&
    (
        cd repo &&
        echo something >somefile &&
        git add somefile &&
        git commit -m "add a file"
    ) &&
    git --git-dir=repo/.git --work-tree=repo rm somefile &&
    test_must_fail git -C repo ls-files --error-unmatch somefile

It's up to you whether you actually want to include such a follow-up
patch in your series; it's certainly not a requirement.