On Mon, Sep 10, 2018 at 5:52 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
> >> Of course, even though these are 1/2 and 2/2, only one of them and
> >> not both would apply.
> >
> > Or you could squash them once we reach consensus that both are good.
>
> Ah, sorry, I completely misread the first one. I thought that it
> was extending the implementation of existing unused function by
> using trace API, which explains why I mistook them as an either-or
> choice. I did not realize 1/2 was adding yet another unused one
> without doing anything to the existing unused one.
>
> So the choice being offered are:
>
> (0) take 2/2 only, keeping zero unused helper.
> (1) take 1/2 only, keeping two unused helpers.
> (2) do nothing, keeping the simple unused helper we had from the
> beginning of time.
> (3) take 1/2 and 2/2, replacing one simple unused helper with
> another unused helper that is more complex and capable.
>
> Are you planning to, or do you know somebody who plans to, use one
> or the other if available in a near future? If so, it would be OK
> to take choice (2) or (3), and it probably is preferrable to take
> (3) between them. A more complex and capable one would require
> maintenance over time (trace API is being updated with the trace2 in
> another topic that will start flying soon, so it would be expected a
> user of trace API may need update), but as long as that is actually
> used and help developers, that maintenance cost is worth paying.
>
> If not, I would say that the option (1) or (3) are worse choices
> than either (0) or (2). It would be better to minimize maintenance
> cost for unused helper(s), and the simpler one is likely to stay
> maintenance free for longer than the more complex and capable one,
> so (1) and (3) do not make much sense unless we plan to start using
> these real soon.
Yes, I think (0) is the way to go, actually.
I wrote patch 1/2 to show Peff and you to prove otherwise that I am
not contributing "only grudgingly".
If the current unused function would be actually helpful in debugging
I would not remove it, but actually use it.
>
> >> It is not costing us much to leave it in the code. It's not like
> >> the function costed a lot of maintenance burden since it was added
> >> in 8fd2cb40 ("Extract helper bits from c-merge-recursive work",
> >> 2006-07-25), so the alternative #3 might be to do nothing.
> >
> > True, but ...
> >
> >> somebody else in the future to propose removing
> >
> > is what is actually happening here already, see
> >
> > https://public-inbox.org/git/1421343725-3973-1-git-send-email-kuleshovmail@xxxxxxxxx/
> >
> >> I am inclined to say we'd take
> >> 2/2 ;-)
> >>
> >> Thanks.
> >
> > Feel free to take Alexanders patch from 2015 instead.
>
> I prefer to take 2/2 over the one from 2015, especially if we can
> explain the removal better. We had three extra years that the
> helper stayed unused and unchanged, which gives us a better
> indication that it won't be missed.
Will send a patch with better reasons tomorrow,
Thanks,
Stefan
