Re: [PATCH 2/8] midx-write: add repository field to `write_midx_context`

From: Patrick Steinhardt <ps@pks.im>
Date: Nov 18, 2024, 08:05 UTC
Message-ID: <Zzr1ZMmH9p8BF4O6@pks.im>
In-Reply-To: <20241115-374-refactor-midx-c-and-midx-write-c-to-not-depend-on-global-state-v1-2-761f8a2c7775@gmail.com>

On Fri, Nov 15, 2024 at 02:42:15PM +0100, Karthik Nayak wrote:

Show 13 quoted lines

> @@ -651,9 +653,10 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,
>  	struct strbuf buf = STRBUF_INIT;
>  	char *tmp_file;
>  
> -	trace2_region_enter("midx", "write_midx_reverse_index", the_repository);
> +	trace2_region_enter("midx", "write_midx_reverse_index", ctx->repo);
>  
> -	strbuf_addf(&buf, "%s-%s.rev", midx_name, hash_to_hex(midx_hash));
> +	strbuf_addf(&buf, "%s-%s.rev", midx_name, hash_to_hex_algop(midx_hash,
> +								    ctx->repo->hash_algo));
>  
>  	tmp_file = write_rev_file_order(NULL, ctx->pack_order, ctx->entries_nr,
>  					midx_hash, WRITE_REV);

I think it would be nice to split up the changes that start to pass down a repo via the context and those that start to actually call a different function like we have here. Otherwise it's hard to spot the callsites where one actually has to think about what's happening.

Patrick