Re: [PATCH v3 0/8] In grep, no adding submodule ODB as alternates

From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date: Aug 17, 2021, 19:29 UTC
Message-ID: <CAHd-oW6Va31PaTQeoSrwi09wjHk63S-Xd_PYtW1trhSokoKizQ@mail.gmail.com>
In-Reply-To: <cover.1629148153.git.jonathantanmy@google.com>

On Mon, Aug 16, 2021 at 6:10 PM Jonathan Tan <jonathantanmy@google.com> wrote:

>
> Thanks for reviewing, everyone. Here are the requested changes.

Thanks. You've addressed all my comments from the previous rounds.

One minor suggestion which is not worth a re-roll on its own:

> Range-diff against v2:

[...]

Show 10 quoted lines

> 7:  94db10a4e5 ! 7:  8b86618531 submodule-config: pass repo upon blob config read
>     @@ Commit message
>          When reading the config of a submodule, if reading from a blob, read
>          using an explicitly specified repository instead of by adding the
>          submodule's ODB as an alternate and then reading an object from
>          the_repository.
>
>     +    This makes the "grep --recurse-submodules with submodules without
>     +    .gitmodules in the working tree" test in t7814 work when
>     +    GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB is true.

This sounds to me as if the test was previously failing when GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB is true, which is not the case. I think an alternative could be:

This code path is exercised by the "grep --recurse-submodules with submodules without .gitmodules in the working tree" test in t7814. The test passes with GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1, showing that we indeed are no longer accessing the submodule's ODB through the alternates list.

But it's really a minor thing. With or without this change:

Reviewed-by: Matheus Tavares <matheus.bernardino@usp.br>