From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 24 Sep 2025 06:18:32 GMT
Subject: Re: [GSoC][PATCH v4 1/9] refs: add a generic 'optimize' API
Message-ID: <aNONOM4W7kUQNm1y@pks.im>
In-Reply-To: <20250919082647.535213-2-meetsoni3017@gmail.com>

On Fri, Sep 19, 2025 at 01:56:39PM +0530, Meet Soni wrote:
> The existing `pack-refs` API is conceptually tied to the 'files'
> backend, but its behavior is generic (e.g., it triggers compaction for
> reftable). This naming is confusing.
> 
> Introduce a new generic refs_optimize() API that dispatches to a
> backend-specific implementation via a new 'optimize' vtable method.
> 
> This lays the architectural groundwork for different reference backends
> (like 'files' and 'reftable') to provide their own storage optimization
> logic, which will be called from a single, generic entry point.

I agree with this change in the architecture in general -- "packing
refs" is certainly a term that is specific to the "files" backend. So
renaming that infrastructure to instead say "optimizing refs" feels like
a sensible step as it adjusts naming to reality.

But what I don't quite get is why we end up with both a `pack_refs_fn`
and an `optimize_fn` after this series, where the latter is always
calling the former. Wouldn't it be more sensible step to make this a
couple of simple renames? E.g.:

  - `pack_refs_fn` -> `optimize_refs_fn`
  - `refs_pack_refs()` -> `refs_optimize()`
  - `struct pack_refs_opts` -> `struct refs_optimize_opts`

It would probably be a bit of the bigger patch to do all these renames
at once. But there aren't _that_ many users of this infra, and I'd quite
welcome those changes.

Patrick