From: Toon Claes Date: Thu, 09 Oct 2025 10:11:31 GMT Subject: Re: [PATCH v2 00/14] refs: improvements and fixes for peeling tags Message-ID: <87ecrcbd0s.fsf@iotcl.com> In-Reply-To: <20251009063956.GA1622884@coredump.intra.peff.net> Jeff King writes: >> > 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. When I was reviewing these patches, I also had some doubts whether `reference` was the best name for this struct. Especially because the comment above says: /* A reference passed to `for_each_ref()`-style callbacks. */ But on the other hand, I agree with Patrick: >> what it contains is as close as you get to a representation of a >> reference. So, I'd be fine if this comment was rewritten a little bit, into something like: /* A representation of a reference, for example to be passed to * `for_each_ref()`-style callbacks. */ > 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. Personally I'd say it's hard to predict something like that. Usually I tend to lean toward naming that doesn't assume anything that might happen in the future. And because I think this struct contains what a reference _is_, I think this name is fine. And whenever we run into the issue that we need something else to be named `reference`, we revisit this decision. > 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). -- Cheers, Toon