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 [off-list ref] wrote:
On Thu, Jun 10, 2021 at 10:35 AM Jonathan Tan [off-list ref] wrote:
quoted
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.