From: Jeff King <peff@peff.net>
Date: Thu, 09 Oct 2025 05:38:25 GMT
Subject: Re: [PATCH v2 00/14] refs: improvements and fixes for peeling tags
Message-ID: <20251009053825.GB1614343@coredump.intra.peff.net>
In-Reply-To: <20251008-b4-pks-ref-filter-skip-parsing-objects-v2-0-76e30d5c9542@pks.im>

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.

-Peff