Re: [PATCH 00/49] repack: prepare for incremental MIDX-based repacking

Jeff King
2025-10-15T11:06:54Z

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