Re: [PATCH 1/2] trace: add trace_print_string_list_key

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

>> 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.

Thanks.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux