On Thu, Mar 11, 2021 at 12:04:36PM -0500, Taylor Blau wrote: > Subcommands of the 'git multi-pack-index' command (e.g., 'write', > 'verify', etc.) will want to optionally change a set of shared flags > that are eventually passed to the MIDX libraries. > > Right now, options and flags are handled separately. Inline them into > the same structure so that sub-commands can more easily share the > 'flags' data. This "opts" struct is kind of funny. It is used to collect the options in cmd_multi_pack_index(), but nobody ever passes it anywhere! Instead, we pass individual components of it around. So I'm not sure I buy "...so that sub-commands can more easily share the flags data", since either way they are all receiving the individual flags field already. And your patch 2 could just as easily do the same simplification by modifying the function-local "flags" variable. But. I think things get more interesting when you later introduce common_opts, because now those options have to refer back to actuals storage for each item. Which means that "flags" would have to become a global variable. And there it's nicer to have all of the options stuffed into a struct, even if it is a single global struct. So I think this is the right direction, but it took me a minute to realize quite why. -Peff
