Subject: Re: [PATCH 00/20] parse-options: handle subcommands

From: Derrick Stolee <[email-masked]>
Date: Mon, 25 Jul 2022 12:08:16 -0400

On 7/25/2022 12:00 PM, SZEDER Gábor wrote:
> On Mon, Jul 25, 2022 at 09:15:45AM -0400, Derrick Stolee wrote:
>> I focused on reading the changes to the builtins I have experience with
>> (commit-graph, maintenance, multi-pack-index, sparse-checkout, worktree)
>> and found the adaptation to the new model very clean.
>>
>> The one common thing I saw was that you are updating a function pointer
>> that you name "fn" but it could be more informative on first reading if
>> it was named something like "subcommand_fn".
> 
> I felt that redundant, because most lines mentioning that 'fn'
> have something clearly subcommand-specific next to it, i.e. the type
> 'parse_opt_subcommand_fn' at its declaration, or the OPT_SUBCOMMAND
> macro.
 
I guess I was just reading the final "return fn(...);" at the end
and thought it looked a bit generic. It's probably not worth
changing.

Thanks,
-Stolee