Re: [PATCH v2 2/2] clean: new option --exclude-from

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 7, 2015, 22:53 UTC
Message-ID: <CAPig+cQb754exY9KTTyohTqoin_uzRspQwrmeoDeJ5bQ=0jzLQ@mail.gmail.com>
In-Reply-To: <1449413906-23256-2-git-send-email-rouzier@gmail.com>

In addition to Peff's and Junio's review comments...

On Sun, Dec 6, 2015 at 9:58 AM, James <rouzier@gmail.com> wrote:

> From: James Rouzier <rouzier@gmail.com>
>
> Specify a file to read for exclude patterns.

Missing Signed-off-by:.

Show 7 quoted lines

> ---
> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
> @@ -628,6 +628,66 @@ test_expect_success 'git clean -e' '
> +test_expect_success 'git clean --exclude-from' '
> +       rm -fr repo &&
> +       mkdir repo &&
> +       cd repo &&

See my review comments for patch 1/2 as to why you want to wrap 'cd' and remaining statements in a subshell.

> +       git init &&
> +       touch known 1 2 3 &&

Likewise, use '>' rather than 'touch' to create empty files when the timestamp isn't significant.

   >1 &&
   >2 &&
   >3 &&

Show 39 quoted lines

> +       git add known &&
> +       cat >.git/clean-exclude <<-\EOF &&
> +       1
> +       2
> +       EOF
> +       git clean -f --exclude-from=.git/clean-exclude &&
> +       test_path_is_file 1 &&
> +       test_path_is_file 2 &&
> +       test_path_is_missing 3 &&
> +       test_path_is_file known
> +'
> +
> +test_expect_success 'git clean -e --exclude-from' '
> +       rm -fr repo &&
> +       mkdir repo &&
> +       cd repo &&
> +       git init &&
> +       touch known 1 2 3 &&
> +       git add known &&
> +       echo 1 >> .git/clean-exclude &&
> +       git clean -f -e 2 --exclude-from=.git/clean-exclude &&
> +       test_path_is_file 1 &&
> +       test_path_is_file 2 &&
> +       test_path_is_missing 3 &&
> +       test_path_is_file known
> +'
> +
> +test_expect_success 'git clean --exclude-from --exclude-from' '
> +       rm -fr repo &&
> +       mkdir repo &&
> +       git init &&
> +       touch known 1 2 3 &&
> +       git add known &&
> +       cat >.git/clean-exclude1 <<-\EOF &&
> +       1
> +       EOF
> +       cat >.git/clean-exclude2 <<-\EOF &&
> +       2
> +       EOF

Creation of these single-line files probably would be more readable using 'echo', as you do in the test just above (for .git/clean-exclude):

    echo 1 >.git/clean-exclude1 &&
    echo 2 >.git/clean-exclude2 &&

Show 20 quoted lines

> +       git clean -f --exclude-from=.git/clean-exclude1 --exclude-from=.git/clean-exclude2 &&
> +       test_path_is_file 1 &&
> +       test_path_is_file 2 &&
> +       test_path_is_missing 3 &&
> +       test_path_is_file known
> +'
> +
> +test_expect_success 'git clean --exclude-from=BADFILE' '
> +       rm -fr repo &&
> +       mkdir repo &&
> +       cd repo &&
> +       git init &&
> +       test_expect_code 128 git clean -f --exclude-from=.git/clean-exclude-not-there
> +'
> +
>  test_expect_success SANITY 'git clean -d with an unreadable empty directory' '
>         mkdir foo &&
>         chmod a= foo &&
> --
> 2.3.6