From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 10 Oct 2025 11:22:10 GMT
Subject: Re: [PATCH 1/9] refs: move to using the '.optimize' functions
Message-ID: <aOjsYoeMkF6-w7G7@pks.im>
In-Reply-To: <20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-1-c7962be584fa@gmail.com>

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.

> 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."

> 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.

Patrick