Re: [PATCH v4 0/8] Repack objects into separate packfiles based on a filter

From: Taylor Blau <me@ttaylorr.com>
Date: Aug 9, 2023, 21:45 UTC
Message-ID: <ZNQI5vpcoXbkkPWa@nand.local>
In-Reply-To: <20230808082608.582319-1-christian.couder@gmail.com>

On Tue, Aug 08, 2023 at 10:26:00AM +0200, Christian Couder wrote:

> # Changes since version 3
>
> Thanks to Junio who reviewed both version 1, 2 and 3, and to Taylor
> who reviewed version 1 and 3! The changes are the following:

I took a look through the range-diff as well as the patches themselves again (skimming through the last three, which are much more straightforward than the preceding ones).

Everything looks good to me here, and I think that this version is ready to get picked up once we're on the other side of 2.42.

I left a couple of comments throughout, but none of them merit a reroll on their own. I think there are a couple of things we could easily ignore (marking parameters as "const", etc.), and a couple of things that we should probably take a look at after the dust has settled here.

We *may* want to fix up the test_must_fail invocation that has the environment variable on the left-hand side instead of using "test_must_fail env", but I don't know for sure.

I do think that we should take another look at disabling the bitmap machinery when given `--filter`, but I think that, too, can be done in another series.

Thanks again for being so patient with all of my review comments. I hope it wasn't too big of a pain; this area feels very fragile (to me, at least) so I wanted to give it an extra careful set of eyes.

Thanks, Taylor