git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/9] fetch: pass through `fetch_config` directly

From
Jeff King <peff@peff.net>
Date
May 19, 2023, 00:18 UTC
Message-ID
<20230519001803.GC2442034@coredump.intra.peff.net>
In-Reply-To
<b09f21dff786780dfb0ae71efd9a0cabe31c1e0b.1684324059.git.ps@pks.im>
On Wed, May 17, 2023 at 01:48:51PM +0200, Patrick Steinhardt wrote:
Show 6 quoted lines
> 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:
Show 14 quoted lines
>  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
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 7 of 16 in “fetch: smallish cleanups”
  1. 0/9 fetch: smallish cleanupsPatrick Steinhardt, May 17, 2023
  2. 1/9 fetch: drop unused DISPLAY_FORMAT_UNKNOWN enum valuePatrick Steinhardt, May 17, 2023
  3. Jeff KingMay 19, 2023
  4. 2/9 fetch: drop unneeded NULL-check for `remote_ref`Patrick Steinhardt, May 17, 2023
  5. Jeff KingMay 19, 2023
  6. 3/9 fetch: pass through `fetch_config` directlyPatrick Steinhardt, May 17, 2023
  7. Jeff KingMay 19, 2023
  8. Patrick SteinhardtMay 22, 2023
  9. Jeff KingMay 22, 2023
  10. 4/9 fetch: use `fetch_config` to store "fetch.prune" valuePatrick Steinhardt, May 17, 2023
  11. Jeff KingMay 19, 2023
  12. 5/9 fetch: use `fetch_config` to store "fetch.pruneTags" valuePatrick Steinhardt, May 17, 2023
  13. 6/9 fetch: use `fetch_config` to store "fetch.showForcedUpdates" valuePatrick Steinhardt, May 17, 2023
  14. 7/9 fetch: use `fetch_config` to store "fetch.recurseSubmodules" valuePatrick Steinhardt, May 17, 2023
  15. 8/9 fetch: use `fetch_config` to store "fetch.parallel" valuePatrick Steinhardt, May 17, 2023
  16. 9/9 fetch: use `fetch_config` to store "submodule.fetchJobs" valuePatrick Steinhardt, May 17, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.