Re: [PATCH v2 00/14] refs: improvements and fixes for peeling tags
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Oct 10, 2025, 05:22 UTC
- Message-ID
- <aOiYFPTNLL1Fgz5V@pks.im>
- In-Reply-To
- <20251010051242.GC1897715@coredump.intra.peff.net>
On Fri, Oct 10, 2025 at 01:12:42AM -0400, Jeff King wrote:
Show 47 quoted lines
> On Thu, Oct 09, 2025 at 09:24:35AM +0200, Patrick Steinhardt wrote: > > > > 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). > > > > 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.
I think I'd like to avoid doing such a swap in this series, as it may easily cause confusion and create problems for any in-flight series. From my point of view I also think that neither of these names is clearly superior over the other.
So to move forward, how about we land this as-is and I promise to follow up with another series that:
- Renames `struct ref` as proposed.
- Introduces `struct reference` into more of our APIs?
Thanks!
Patrick