On Thu, Aug 9, 2018 at 2:16 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Stefan Beller <sbeller@xxxxxxxxxx> writes: > > > It is a debugging aid, so it should print to the debugging channel. > > Who says? The comment in the header file? /** * Dump a string_list to stdout, useful mainly for debugging * purposes. It can take an optional header argument and it writes out * the string-pointer pairs of the string_list, each one in its own * line. */ > In-tree code may say so, and I do not think any in-flight > topic up to 'pu' uses this to produce non-debugging output, but I do > not think it is healthy attitude to assume that you can take over an > existing function and change what it does unilaterally. That is reasonable, as usual, and given the recent fallout of the object store refactoring I have these concerns on my mind, too. But for this instance, I do not see any risk of accidental collisions with other series in flight: $ git log -S print_string_list origin/pu --oneline 4f665f2cf33 string-list.h: move documentation from Documentation/api/ into header d10cb3dfab9 help.c: rename function "pretty_print_string_list" c455c87c5cd Rename path_list to string_list 1eb056905a2 include $PATH in generating list of commands for "help -a" 70827b15bfb Split up builtin commands into separate files from git.c 27dedf0c3b7 (tag: v1.0rc3, tag: v0.99.9j) GIT 0.99.9j aka 1.0rc3 8e49d503882 C implementation of the 'git' program, take two. > As there is no in-tree or in-flight user, I think it makes sense if > the proposed change were to rename it to trace_string_list(). If > there weren't any print_string_list() and we were adding a debugging > aid to use trace_printf() to dump these, that would be the name we > would use anyway. Good call. I'll rename and send separately (or might even drop this patch completely; this patch is not used in the patches to follow; it is more a "FYI: here is a thing that I found helpful for debugging, unlike the existing function, which I found unhelpful and actively harmful")
