Re: [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt()

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 20, 2022, 23:50 UTC
Message-ID: <xmqqilkeoy0q.fsf@gitster.g>
In-Reply-To: <84971a3a-4c5c-5b79-582e-88b3bd8c624f@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 6 quoted lines

> I wrote that patch out of curiosity and was pleased by the total line
> count reduction, getting rid of the on-stack array construction sites
> with their magic numbers, not having to manage strvecs explicitly
> anymore and the callers staying readable.
> ...
> But still: Is all of that code churn worth it?  Not sure.

Compared to doing nothing? The result did look easier to grok, especially as we no longer need to worry about another way to do the same thing (i.e. run_command() vs run_command_v_*() variants) and can get rid of the flags constants that need to be kept in sync with the members of the child_process struct.

Compared to adding the _l_opt() variant? Surely, as that goes the other direction to add more callers that use the flags constants, without much practical gain (which was a bit sad, but we only found that out after seeing the result).