Re: [PATCH 01/20] packfile: pass down repository to `odb_pack_name`
- From
karthik nayak <karthik.188@gmail.com>
- Date
- Oct 22, 2024, 08:51 UTC
- Message-ID
- <CAOLa=ZQPH_rd65-zW6dRJAiAA747htXR---icS5aV9zupH2aGw@mail.gmail.com>
- In-Reply-To
- <ZxbCWvFo8ZdwPgJd@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
Show 28 quoted lines
> On Mon, Oct 21, 2024 at 11:57:44AM +0200, Karthik Nayak wrote:
>> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
>> index 5809613002..60f806e672 100644
>> --- a/builtin/pack-redundant.c
>> +++ b/builtin/pack-redundant.c
>> @@ -688,7 +688,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, s
>> pl = red = pack_list_difference(local_packs, min);
>> while (pl) {
>> printf("%s\n%s\n",
>> - sha1_pack_index_name(pl->pack->hash),
>> + sha1_pack_index_name(the_repository, pl->pack->hash),
>> pl->pack->pack_name);
>> pl = pl->next;
>> }
>
> I am a little surprised to see sha1_pack_index_name() converted
> similarly here, as this patch promises only to touch the
> 'odb_pack_name()' function.
>
>> diff --git a/packfile.h b/packfile.h
>> index 0f78658229..507ac602b5 100644
>> --- a/packfile.h
>> +++ b/packfile.h
>
> Indeed, it looks like odb_pack_name(), sha1_pack_name(),
> sha1_pack_index_name(), parse_pack_index(), and has_pack_index() are all
> modified. Were these meant to go in separate patches?
>Nope this is intentional, each commit tries to pick a base function and modifies all layers above it (I should have explicitly specified my approach in the cover).
In this commit, we try to modify `odb_pack_name()` and therefore sha1_pack_name(), sha1_pack_index_name(), parse_pack_index(), and has_pack_index() too. Otherwise, we'd have a lot lot more commits.
This simplies the review too, but yes, will add more instructions to the next version.
> Thanks, > Taylor
Karthik