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.