Re: [PATCH 2/2] Added test from David Glasser.

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Sep 9, 2010, 21:33 UTC
Message-ID: <vpqk4muvbyt.fsf@bauges.imag.fr>
In-Reply-To: <1284056514-4935-2-git-send-email-davi.reis@gmail.com>

davi.reis@gmail.com writes:

Show 8 quoted lines

> --- a/builtin/ls-tree.c
> +++ b/builtin/ls-tree.c
> @@ -52,7 +52,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
>  		speclen = strlen(spec);
>  		if (speclen <= len)
>  			continue;
> -		if (spec[len] != 0 && spec[len] != '/')
> +		if (spec[len] != '/')

This change is not the one advertized for in the title. If you didn't mean it, then

  git send-email --annotate

can be your friend, it gives you a last opportunity to check your patch before it is sent.

If you did mean it, then it should be justified in the commit message.

Show 16 quoted lines

> --- a/t/t3100-ls-tree-restrict.sh
> +++ b/t/t3100-ls-tree-restrict.sh
> @@ -165,4 +165,13 @@ test_expect_success \
>  EOF
>       test_output'
>  
> +test_expect_success \
> +    'ls-tree with one path a prefix of the other' \
> +    'git ls-tree $tree path2/baz path2/bazbo >current &&
> +     make_expected <<\EOF &&
> +040000 tree X  path2/baz
> +120000 blob X  path2/bazbo
> +EOF
> +     test_output'
> +
>  test_done

Adding the test can help people to understand what the first patch is fixing, hence, I'd suggest either squashing both patches, or putting the test patch first (with a test_expect_failure), and having the second turn the test_expect_failure into a test_expect_success (hence, it's obvious reading the patch that it fixes the test).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/