From: Karthik Nayak <karthik.188@gmail.com>
Date: Mon, 13 Oct 2025 08:18:24 GMT
Subject: Re: [PATCH 1/9] refs: move to using the '.optimize' functions
Message-ID: <CAOLa=ZRuZebNnPz9punnHxBqSPNp+CbM4FJ6BG1RwC1D_Fjz0w@mail.gmail.com>
In-Reply-To: <aOjsYoeMkF6-w7G7@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Oct 10, 2025 at 12:27:05PM +0200, Karthik Nayak wrote:
>> The `struct ref_store` variable, exposes two ways to optimize a reftable
>> backend:
>>
>>   1. pack_refs
>>   2. optimize
>>
>> The former was specific to the 'files' + 'packed' refs backend. The
>> latter is more generic and covers all backends. While the naming is
>> different, both these tend to perform the same functionality.
>
> "tend to perform" is a bit of a curious thing to say, as it raises the
> question when it doesn't.
>

Indeed, will rephrase.

>> In the following commit, we will consolidate this code to only maintain
>> the 'optimize' functions. In preparation, modify the backends to also do
>> the same, by moving to supporting the 'optimize' function.
>
> Maybe: "In preparation, modify the backends so that they exclusively
> implement the `optimize` callback, only."
>

This reads better, thanks!

>> diff --git a/refs/files-backend.c b/refs/files-backend.c
>> index a4cda57981..0b81bd7f74 100644
>> --- a/refs/files-backend.c
>> +++ b/refs/files-backend.c
>> @@ -1445,8 +1445,8 @@ static int should_pack_refs(struct files_ref_store *refs,
>>  	return 0;
>>  }
>>
>> -static int files_pack_refs(struct ref_store *ref_store,
>> -			   struct pack_refs_opts *opts)
>> +static int files_optimize(struct ref_store *ref_store,
>> +			  struct pack_refs_opts *opts)
>
> `struct pack_refs_opts` really should be renamed to
> `refs_optimize_opts`. Let's read on, maybe you do this in subsequent
> patches.
>

Yup, that's handled in a follow up patch!

> Patrick