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.