From: Jonathan Tan <jonathantanmy@google.com>
Date: Thu, 23 Dec 2021 19:08:08 GMT
Subject: Re: [PATCH v3 3/3] builtin/fetch: die on --negotiate-only and --recurse-submodules
Message-ID: <20211223190808.1285008-1-jonathantanmy@google.com>
In-Reply-To: <20211222001134.28933-4-chooglen@google.com>

Glen Choo <chooglen@google.com> writes:
> The previous commit ignores the value of --recurse-submodules if
> --negotiate-only is given. Since non "no" values of --recurse-submodules
> are not supported with --negotiate-only, make cmd_fetch() check for
> this invalid combination and die.
> 
> This is unlikely to affect internal usage of --negotiate-only, but it
> may be helpful for users. We may also want to discourage users from
> using --negotiate-only altogether because it was not intended for them.

All 3 patches look good, and I'm happy if they were merged as-is.

Having said that, I would delete the last sentence - I envision that
other tools may want to use --negotiate-only to be able to better query
a Git server.

> @@ -75,6 +75,7 @@ static struct transport *gtransport;
>  static struct transport *gsecondary;
>  static const char *submodule_prefix = "";
>  static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> +static int recurse_submodules_explicit = RECURSE_SUBMODULES_DEFAULT;

I would call this "recurse_submodules_cli" as a config variable could
also be considered explicit.

But again, you can consider all these suggestions as optional.