From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 09 Oct 2025 06:09:53 GMT
Subject: Re: [PATCH v2 00/14] refs: improvements and fixes for peeling tags
Message-ID: <aOdRsR-k77uTWJRb@pks.im>
In-Reply-To: <20251009053825.GB1614343@coredump.intra.peff.net>

On Thu, Oct 09, 2025 at 01:38:25AM -0400, Jeff King wrote:
> On Wed, Oct 08, 2025 at 05:50:15PM +0200, Patrick Steinhardt wrote:
> 
> >   - Patches 1 to 8 refactor our codebase so that we don't have the
> >     `peel_iterated_object()` hack anymore. I just found it hard to
> >     follow and thought it shouldn't be too hard to get rid of it.
> 
> I'm really happy to see this hack go away. I've wanted to fix it for
> ages, but didn't want to bite the bullet on changing all of the
> each_ref_fn callbacks. The approach you used here to pass through a
> struct looks good to me.
> 
> I do have one minor complaint, though: the name of that struct. I have a
> feeling that the name "struct reference" may cause confusion down the
> road because it's so generic, and because "references" and "refs" are so
> common in the code. From the names, when would I know when to use
> "struct reference" and when "struct ref"?
> 
> Could we give it a name that ties it to the iteration interface?
> Something like iterated_ref, each_ref_data, etc?
> 
> I know this is minor (and will be annoying to adjust your series), but
> I'd rather raise the point now than realize later that it's confusing
> and try to change it then.

It is puzzling indeed. I would claim that in this case it is not `struct
reference` that is misnamed: what it contains is as close as you get to
a representation of a reference. It's rather `struct ref` that is
misnamed, as it carries a lot of data that is only valid in the context
of a remote.

Another approach could thus be to rename `struct ref` to `struct
remote_ref`, which I would claim would be a clear win for better semantics.
It's used in lots of places though, which is a valid counter argument.

WDYT?

Patrick