Re: [PATCH 0/9] fetch: further ref-prefix cleanups and optimizations

From: Jeff King <peff@peff.net>
Date: Mar 13, 2025, 05:50 UTC
Message-ID: <20250313055010.GG94015@coredump.intra.peff.net>
In-Reply-To: <Z9IBZmh4Cr2jq7E4@nand.local>

On Wed, Mar 12, 2025 at 05:49:26PM -0400, Taylor Blau wrote:

Show 14 quoted lines

> On Wed, Mar 12, 2025 at 05:29:28PM -0400, Taylor Blau wrote:
> > >  builtin/fetch.c        | 46 +++++++++++++++++-------------------------
> > >  refspec.c              | 22 ++++++++++++++------
> > >  t/t5516-fetch-push.sh  | 12 ++++++-----
> > >  t/t5702-protocol-v2.sh | 44 +++++++++++++++++++++++++++++++++++++++-
> > >  4 files changed, 85 insertions(+), 39 deletions(-)
> >
> > Kidding, of course, I am looking forward to reviewing it now.
> 
> This looks great to me. I left a few minor comments throughout, but am
> unsurprisingly on-board with the overall approach as you and I discussed
> this off-list a week or two ago.
> 
>     Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks. My biggest question is on patch 8, and whether the existing set_head() feature really ought to be more aggressive about pointing at stuff we didn't fetch (and that could even be unborn on our side!).

I don't have a strong opinion either way, but it would mean adjusting the idea of when set_head() kicks in (e.g., should we do it for every fetch, or should exact-oid fetches continue to optimize out the ls-refs call?).

-Peff