Re: [PATCH 01/11] string_list: print_string_list to use trace_printf

From: Stefan Beller <sbeller@google.com>
Date: Sep 7, 2018, 17:21 UTC
Message-ID: <CAGZ79kbObDXyaRLADQpvMMSitOLCDwb030Q6UBiX-7Y-XWnqog@mail.gmail.com>
In-Reply-To: <xmqqo9d9y6oy.fsf@gitster-ct.c.googlers.com>

On Fri, Sep 7, 2018 at 2:53 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

>
> Jeff King <peff@peff.net> writes:
>
> > I guess the question is: is this a thing we would want to make available
> > to code to leave in all the time? Or is it just for sticking in
> > temporarily for a quick dump?
> >
> > If the former, then I think it needs the early-return at the least (and
> > probably _should_ have the key parameter).
> >
> > But I get the feeling that you really just want the latter, and are only
> > grudgingly being pushed into the former by Junio's suggestion.

That is not quite the case. I did take all but the one suggestion as I think they are good suggestions and I appreciate it.

However having the trace key is not useful I would think. (I did not need it for debugging, but presented an API of what I would have found useful)

If the key is really that useful, I would just think we can add it on top.

Show 9 quoted lines

> Well, if that is the case, I'd change "my suggestion" (although I
> didn't mean to suggest anything concrete).  If this was needed
> and/or was useful only during the defvelopment of the remainder of
> the series, and if this is known to be a half-hearted change that is
> not meant to be useful as a general solution, then let's not take
> the main part of the series a hostage to this step; rather, we
> should drop this change and leave it to another series that is
> willing to do it right.  It would save both author's and reviewers'
> time if we did so.

In that case, would we be interested in a separate patch that kills off that function instead?

Stefan