Re: [PATCH 0/8] run-command: remove run_command_v_*()

["René Scharfe" <l.s.r@web.de>]

Am 27.10.22 um 22:11 schrieb Jeff King:
> On Thu, Oct 27, 2022 at 06:30:36PM +0200, René Scharfe wrote:
>
>> Replace the convenience functions run_command_v_opt() et. al. and use
>> struct child_process and run_command() directly instead, for an overall
>> code reduction and a simpler and more flexible API that allows creating
>> argument lists without magic numbers and reduced risk of memory leaks.
>>
>> This is a broken-out and polished version of the original scratch at
>> https://lore.kernel.org/git/9d924a5d-5c72-fbe6-270c-a8f6c5fc5850@web.de/
>
> I read through this and it all looks fine to me. I was a bit puzzled at
> the layout of your series at first. In particular, the difference
> between cases in patch 4 versus the later ones.
>
> I think it is that in patch 4, these are all unambiguously positive
> because we are getting rid of magic numbers (or magically-sized arrays).
> Whereas in patches 5-8, there's nothing inherently wrong with the
> call-sites; but as we get rid of the API wrappers, we convert them. So
> they are collateral damage, so to speak, from the simplification of the
> API.
>
> That makes sense to me, though I could point out that most of the sites
> cleaned up in patch 4 _could_ be converted to look like the ones that
> are converted in 5-8. Obviously that doesn't make sense to do, knowing
> that 5-8 are coming. But if the point in splitting it this way is to
> show that we could stop at patch 4, cleaning up call sites but not
> shrinking the run-command API, then I just want to point out that there
> is another way to do those cleanups. :)

Yes, almost, except that I think 5-7 are doing necessary pruning and 8
requires a small leap of faith in the value of simplicity.  And I wanted
to shrink down and simplify that last patch.

René