From: Jeff King <peff@peff.net>
Date: Thu, 09 Oct 2025 06:39:56 GMT
Subject: Re: [PATCH v2 00/14] refs: improvements and fixes for peeling tags
Message-ID: <20251009063956.GA1622884@coredump.intra.peff.net>
In-Reply-To: <aOdRsR-k77uTWJRb@pks.im>

On Thu, Oct 09, 2025 at 08:09:53AM +0200, Patrick Steinhardt wrote:

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

I am not so much arguing that "struct reference" is misnamed, as that it
is sufficiently generic that people will reach for it when it is not the
appropriate tool. It is for passing the ref data to the iterator
callback, but it probably doesn't make sense in other contexts. Would we
ever expect anybody to declare their own "struct reference" in a local
function? I don't think so.

And yes, "struct ref" suffers somewhat from the same problem. It is
mostly about using refs in one specific space, but the name does not
really help clarify that. I wouldn't mind seeing that improved, but yes,
it would be a noisy patch. I don't know if remote_ref is the right name,
though (the "peer_ref" links mean we store both local and remote refs in
it, IIRC).

-Peff