Saw this series mentioned in "What's cooking" and remembered I didn't give an update. On Thu, Jun 10, 2021 at 2:29 PM Elijah Newren <newren@xxxxxxxxx> wrote: > > On Thu, Jun 10, 2021 at 10:35 AM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: > > > > I think I've addressed all review comments. As for Junio's suggestion > > about also printing the type in the former patch 4 (now patch 5) [1], I > > decided to just leave the code as-is and not also print the type. > > > > The main changes are that patch 1 is somewhat rewritten - we still > > remove the global variable, but we no longer read the > > extensions.partialClone config directly from promisor-remote.c. Instead, > > we store it in struct repository when the format of a repository is > > being verified, and promisor-remote.c merely reads it from there. Patch > > 3 is a new patch that updates the environment variable preparation > > before it is moved in patch 4 (formerly patch 3). > > I've read through all the patches. 2 & 5 look good to me, I had small > nitpicks on 1 & 4, and I'm totally lost on patch 3. Patch 3 is just a > one-liner and it might be fine, but for some reason I can't figure out > the code before or after the patch even after digging around into > other commits and other files to try to get my bearings. Hopefully > someone else can comment on that one. I'm happy with Jonathan and Peff's responses on patch 3; as I mentioned above I just didn't understand the original code before Jonathan's changes. (Perhaps some comments could be added to clarify that code area, but again that's clarifying the code that existed before Jonathan's patch so it doesn't need to be part of his series.) So that only leaves my nitpicks on patches 1 & 4; otherwise the series looks good to me.
