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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 10, 2018, 22:32 UTC
Message-ID: <xmqq36uhrnjk.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180910215831.17608-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> I separated this from the other series, making it into 2 patches:
> This first patch adds tracing for string lists and the next patch that
> removes the unused function from the string list API.
> That way we can decide on these two patches separately if needed.

Of course, even though these are 1/2 and 2/2, only one of them and not both would apply.

Thanks for sticking to the topic.

Given how simple that "dump them to standard output" code is, I am inclined to say that anybody who needs to inspect the contents of string list at various points in the code under development can create one from scratch even if we did not have this implementation, so perhaps 2/2 is a better choice between the two.

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.

I have no strong preference between #2 and #3. The benefit of #2 compared to #3 is that, if we remove it today, there will not be somebody else in the future to come and propose removing the otherwise unused function, which would cost us time to review and discuss, so unless somebody stops me, I am inclined to say we'd take 2/2 ;-)

Thanks.