From: Karthik Nayak <karthik.188@gmail.com>
Date: Mon, 13 Oct 2025 08:52:55 GMT
Subject: Re: [PATCH 3/9] refs: rename 'pack_refs_opts' to 'optimize_refs_opts'
Message-ID: <CAOLa=ZRkKhjnqw+7J8sc=OSJ6Z1POW8Q6U65=-JwzeRRm6YR4A@mail.gmail.com>
In-Reply-To: <aOjscGfX4ns-5qCm@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Oct 10, 2025 at 12:27:07PM +0200, Karthik Nayak wrote:
>> The previous commit removed all references to 'pack_refs()' within
>> the refs subsystem. Continue this cleanup by also renaming
>> 'pack_refs_opts' to 'optimize_refs_opts' and the respective flags
>> accordingly. Keeping the naming consistent will make the code easier to
>> maintain.
>
> Good, this is what I was hoping to see :)
>
>> diff --git a/refs.c b/refs.c
>> index 77dc1ab501..514fb85af2 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2312,7 +2312,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
>>  	refs->gitdir = xstrdup(path);
>>  }
>>
>> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)
>> +int refs_optimize(struct ref_store *refs, struct optimize_refs_opts *opts)
>
> An options struct for a function `do_something()` should typically be
> called `struct do_something_opts`. So we should rename the struct to
> `refs_optimize_opts`.
>

Will do that.

>> diff --git a/refs.h b/refs.h
>> index c6c955d78d..58b222ac02 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -500,15 +500,15 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,
>>
>>  /*
>>   * Flags for controlling behaviour of refs_optimize()
>> - * PACK_REFS_PRUNE: Prune loose refs after packing
>> - * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end
>> - *                 result are decided by the ref backend. Backends may ignore
>> - *                 this flag and fall back to a normal repack.
>> + * OPTIMIZE_REFS_PRUNE: Prune loose refs after packing
>> + * OPTIMIZE_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end
>> + *                     result are decided by the ref backend. Backends may ignore
>> + *                     this flag and fall back to a normal repack.
>>   */
>> -#define PACK_REFS_PRUNE (1 << 0)
>> -#define PACK_REFS_AUTO  (1 << 1)
>> +#define OPTIMIZE_REFS_PRUNE (1 << 0)
>> +#define OPTIMIZE_REFS_AUTO  (1 << 1)
>
> And these would then be called `REFS_OPTIMIZE_PRUNE` etc.
>
> Patrick

Likewise, thanks for pointing it out.