Re: [PATCH 3/9] fetch: pass through `fetch_config` directly
From: Jeff King <hidden>
Date: 2023-05-19 00:19:59
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:
quoted hunk ↗ jump to hunk
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