From: Jeff King <peff@peff.net>
Date: Wed, 15 Oct 2025 11:06:53 GMT
Subject: Re: [PATCH 00/49] repack: prepare for incremental MIDX-based repacking
Message-ID: <20251015110653.GG2250228@coredump.intra.peff.net>
In-Reply-To: <aOmTqt4m/R/YrOGg@nand.local>

On Fri, Oct 10, 2025 at 07:15:54PM -0400, Taylor Blau wrote:

> > I've read up through patch 37. My eyes were starting to glaze over at
> > that point, and it looked like the patches were starting to get more
> > interesting, so I've saved the rest for another session.
> 
> Thanks for the review thus far!

OK, I've finished reading through. The remainder was actually less
complicated than I thought it might be.

I left a few comments, but there's nothing really important. It sounds
like you're doing a re-roll for minor fixes anyway, so you can take or
leave my ponderings as you see fit.

> Yeah, I think we could probably go on for quite a bit longer polishing
> the naming, interfaces, etc. I tried to strike a balance in this series
> to change as little as possible while still ending up with a
> halfway-decent looking repack.h ;-).
> 
> As you noted, the repack code is extremely fragile/brittle. That is a
> big part of why I both (a) took such an incremental approach as in this
> series and (b) tried to avoid making too many changes as I moved code
> around.
> 
> I think that (b) especially leaves us with an opportunity to clean these
> interfaces up after this series has landed, and I intend on doing so. My
> hope is that this series gives us a solid foundation to make those
> changes on, as opposed to further hacking up builtin/repack.c ;-).

OK, that all sounds good to me. I think even if you never circle back to
clean up further, the end result here is a net improvement.

-Peff