On 24/11/22 10:46AM, Junio C Hamano wrote: > Justin Tobler <jltobler@xxxxxxxxx> writes: > > > When `fetch_pack_config()` is invoked, fetch-pack configuration is > > parsed from the config. As part of this operation, fsck message severity > > configuration is assigned to the `fsck_msg_types` global variable. This > > is optionally used to configure the downstream git-index-pack(1) when > > the `--strict` option is specified. > > > > In a subsequent commit, the same fetch-pack fsck message configuration > > needs to be reused. To facilitate this, introduce `fetch_pack_options` > > which gets written to during the `fetch_pack_config_cb()` instead of > > directly modifying the `fsck_msg_types` global. > > It is unclear how it facilitates to replace one global with another > global that has the data that was previously global as one of its > members. With the above I was somehow expecting that the option > struct instance is allocated on the stack of a function common to > both callers of the configuration reader (i.e. fetch_pack_config()) > as well as the configuration user (i.e. get_pack()). If we were to > allow the latter to keep accessing the global (which is perfectly > fine), wouldn't it be sufficient for the purpose of this series > (which I am imagining wants to call fetch_pack_config() from the > sideways and grab what came from the configuration) to just pass the > fsck_msg_types strbuf through the call chain of the reaading side? For the purposes of this series, the addition of the `fetch_pack_options` structure as a wrapper around `fsck_msg_types` is not needed. As mentioned, it would be sufficient to just pass the `strbuf` directly and have it modified by `fetch_pack_config_cb()`. The original intent of providing the shell structure was to allow for more easy extension of the fetch-pack config parsing in the future, but it doesn't probably make much sense to do it now and its purpose wasn't explained. In the next version I'll drop the use of shell structure in favor of passing an instance of `strbuf` directly. -Justin
