From: Davi Reis Date: Thu, 09 Sep 2010 21:44:21 GMT Subject: Re: [PATCH 2/2] Added test from David Glasser. Message-ID: In-Reply-To: I will squash the patches and try again from scratch. Thanks Matthieu. On Thu, Sep 9, 2010 at 2:33 PM, Matthieu Moy wrote: > davi.reis@gmail.com writes: > >> --- 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. > >> --- 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/ > -- []s Davi de Castro Reis