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

From: karthik nayak <karthik.188@gmail.com>
Date: Nov 18, 2024, 15:37 UTC
Message-ID: <CAOLa=ZSevO8HmKCWbZqPJzLs9eA6Lb942BP6h_=1iWjzETeihA@mail.gmail.com>
In-Reply-To: <Zzr1ZMmH9p8BF4O6@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 21 quoted lines

> On Fri, Nov 15, 2024 at 02:42:15PM +0100, Karthik Nayak wrote:
>> @@ -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

Fair enough, I've split it into 3 separate commits for the next version:

- To pass down `the_repository` to non-static functions
- To use `revs-repo` in `read_refs_snapshot`
- To add repository to `write_midx_context`

This should make it easier for reviewers. Thanks