Re: [PATCH v3] clone, submodule: pass partial clone filters to submodules

From: Jonathan Tan <jonathantanmy@google.com>
Date: Feb 9, 2022, 22:44 UTC
Message-ID: <20220209224406.495563-1-jonathantanmy@google.com>
In-Reply-To: <690d2316ad518ea4551821b2b3aa652996858475.1644034886.git.steadmon@google.com>

Josh Steadmon <steadmon@google.com> writes:

Show 9 quoted lines

>  Documentation/config/clone.txt     |  5 ++++
>  Documentation/git-clone.txt        |  7 ++++-
>  Documentation/git-submodule.txt    |  6 ++++-
>  builtin/clone.c                    | 36 ++++++++++++++++++++++++--
>  builtin/submodule--helper.c        | 30 +++++++++++++++++++---
>  git-submodule.sh                   | 17 ++++++++++++-
>  t/t5617-clone-submodules-remote.sh | 41 ++++++++++++++++++++++++++++++
>  t/t7814-grep-recurse-submodules.sh | 41 ++++++++++++++++++++++++++++++
>  8 files changed, 175 insertions(+), 8 deletions(-)

Thanks for this patch. "clone" currently calls "submodule update" in order to perform the clone in the submodule, and "submodule update" then calls "submodule--helper", so I would expect changes in all 3 files. Looking at the summary above, that indeed is the case.

Show 5 quoted lines

> @@ -544,4 +544,45 @@ test_expect_failure 'grep saves textconv cache in the appropriate repository' '
>  	test_path_is_file "$sub_textconv_cache"
>  '
>  
> +test_expect_success 'grep partially-cloned submodule' '

[snip]

> +		# Verify that we actually fetched data from the promisor remote:
> +		grep \"category\":\"promisor\",\"key\":\"fetch_count\",\"value\":\"1\" trace2.log >/dev/null

No need to redirect to /dev/null, but probably not worth a reroll on its own.

This patch looks good to me.
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>