Re: [PATCH v2 00/14] refs: improvements and fixes for peeling tags
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Oct 9, 2025, 06:09 UTC
- 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:
Show 23 quoted lines
> 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