Re: [PATCH v4 1/2] describe: setup working tree for --dirty

[Eric Sunshine <sunshine@sunshineco.com>]

On Sat, Feb 2, 2019 at 5:05 AM Sebastian Staudt <koraktor@gmail.com> wrote:
> Am Fr., 1. Feb. 2019 um 21:12 Uhr schrieb Eric Sunshine
> <sunshine@sunshineco.com>:
> > On Fri, Feb 1, 2019 at 8:55 AM Sebastian Staudt <koraktor@gmail.com> wrote:
> > > diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> > > @@ -145,14 +145,38 @@ check_describe A-* HEAD
> > > +test_expect_success 'describe --dirty with --work-tree' '
> > > +       [...]
> > > +'
> > > +test_expect_success 'describe --dirty with --work-tree' '
> > > +       [...]
> > > +'
> >
> > Can you give these two new tests different titles to make it easier to
> > narrow down a problem to one or the other if one of them does fail?
> > Perhaps the second test could be titled:
> >
> >     test_expect_success 'describe --dirty with dirty --work-tree' '
> >
> > or something.
>
> Thanks, didn‘t notice this.
> I‘d use a suffix (dirty) for my test titles. But this won‘t work for tests
> using check_describe(). Any objections?

I have no objections to using suffix "(dirty)".

It's true that there are a few duplicate test titles due to
check_describe() invocations, however, this patch isn't introducing
any new callers, so it's not strictly a concern of this series. If you
did want to address that issue, one possibility would be to add a
'title' argument to check_describe() and adjust callers to use a
unique title, however, such a change would not be part of this
particular patch, and I'm not convinced that it's even worth the
effort and churn at this point.