Re: [PATCH 19/19] submodule: don't add submodule as odb for push

From: Jonathan Tan <jonathantanmy@google.com>
Date: Oct 19, 2018, 20:39 UTC
Message-ID: <20181019203937.111225-1-jonathantanmy@google.com>
In-Reply-To: <20181016233550.251311-20-sbeller@google.com>

Show 5 quoted lines

> In push_submodule(), because we do not actually need access to objects
> in the submodule, do not invoke add_submodule_odb().
> (for_each_remote_ref_submodule() does not require access to those
> objects, and the actual push is done by spawning another process,
> which handles object access by itself.)

The code looks good - my analysis is the same as that in my review of the previous version [1].

Can you mention, in the commit message, the tests that exercise the functionality here (and say that they still pass)?

[1] https://public-inbox.org/git/20181011230028.200488-1-jonathantanmy@google.com/

Show 23 quoted lines

> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  submodule.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/submodule.c b/submodule.c
> index 7305ae2e10..e623e6bf7f 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1024,9 +1024,6 @@ static int push_submodule(const char *path,
>  			  const struct string_list *push_options,
>  			  int dry_run)
>  {
> -	if (add_submodule_odb(path))
> -		return 1;
> -
>  	if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
>  		struct child_process cp = CHILD_PROCESS_INIT;
>  		argv_array_push(&cp.args, "push");
> -- 
> 2.19.0
>