Re: [PATCH v2 00/14] refs: improvements and fixes for peeling tags

From: Jeff King <peff@peff.net>
Date: Oct 10, 2025, 05:12 UTC
Message-ID: <20251010051242.GC1897715@coredump.intra.peff.net>
In-Reply-To: <aOdjM8F6WvTEBIo_@pks.im>

On Thu, Oct 09, 2025 at 09:24:35AM +0200, Patrick Steinhardt wrote:

Show 16 quoted lines

> > 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.
> 
> Hm. The thing is: if `struct reference` is established in our code base,
> and if it is a simple representation of a reference, then I think it
> might even be a good thing to having it. I could for example see that we
> gain more interfaces over time that use it.
> 
> For example, functions like `refs_read_ref()` could totally be adapted
> to use the same struct, and I would claim that this is a good thing.
> We'd basically have a single central structure that allows us to get a
> reference out of the "refs" subsystem with metadata.

I agree that if it became the standard representation, then having the generic name would be good. I guess I'm just skeptical that it will become/remain that, and not grow gross appendages like the fetch/push status fields of "struct ref". But maybe I am just too pessimistic. ;)

I do agree that it would be nice for refs_read_ref() and other refstore functions to use this as a common type for returning results. Even if it later gained more fields that were specific to the ref subsystem, they'd still make sense in that context.

So I dunno. I could go either way (keeping your series as-is, or using a new name).

Show 10 quoted lines

> > 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).
> 
> If we decide to do it it should definitely be a standalone patch (or
> patch series). I'm also open for different naming suggestions, but for
> now this is the best I could come up with.

That makes sense. The only reason to touch it here would be if we wanted to free up the name "struct ref" to use right now. I think that is a better name than "struct reference", but given all of the existing uses of "struct ref", it is probably not worth the hassle to switch now.

-Peff