Re: [PATCH 1/9] refs: move to using the '.optimize' functions
Patrick Steinhardt <ps@pks.im> writes:
Show 14 quoted lines
> 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.
>
Show 7 quoted lines
>> 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!
Show 17 quoted lines
>> 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
Karthik Nayak <karthik.188@gmail.com>