On Wed, May 17, 2023 at 01:48:51PM +0200, Patrick Steinhardt wrote:
> The `fetch_config` structure currently only has a single member, which
> is the `display_format`. We're about extend it to contain all parsed
> config values and will thus need it available in most of the code.
>
> Prepare for this change by passing through the `fetch_config` directly
> instead of only passing its single member.
Makes sense.
One small nit:
> static int do_fetch(struct transport *transport,
> struct refspec *rs,
> - enum display_format display_format)
> + const struct fetch_config *config)
> {
> struct ref_transaction *transaction = NULL;
> struct ref *ref_map = NULL;
> @@ -1639,7 +1639,8 @@ static int do_fetch(struct transport *transport,
> if (retcode)
> goto cleanup;
>
> - display_state_init(&display_state, ref_map, transport->url, display_format);
> + display_state_init(&display_state, ref_map, transport->url,
> + config->display_format);
If the point is that fetch_config may start carrying new information,
wouldn't we want to pass it as a whole down to display_state_init()? It
might eventually want to see some of that other config, too.
It's presumably academic for now, and it would not be too hard to change
later if needed, so I don't know that it's worth a re-roll. I just found
it especially funny here since the purpose of the patch is to treat the
config struct as a single unit.
-Peff
