Re: [PATCH 4/4] doc: be more precise on (fetch|pull).recurseSubmodules

[Philippe Blain <levraiphilippeblain@gmail.com>]

The commit message title mentions fetch and pull but the patch also talks about push, maybe add that to the title ?

> Le 28 févr. 2020 à 05:35, Damien Robert <damien.olivier.robert@gmail.com> a écrit :
> 
> The default value also depends on the value of submodule.recurse
> 
> Signed-off-by: Damien Robert <damien.olivier.robert+git@gmail.com>
> ---
> Documentation/config/fetch.txt | 6 ++++--
> Documentation/config/push.txt  | 3 +++
> 2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
> index f11940280f..8f716809f9 100644
> --- a/Documentation/config/fetch.txt
> +++ b/Documentation/config/fetch.txt
> @@ -2,10 +2,12 @@ fetch.recurseSubmodules::
> 	This option can be either set to a boolean value or to 'on-demand'.
> 	Setting it to a boolean changes the behavior of fetch and pull to
> 	unconditionally recurse into submodules when set to true or to not
> -	recurse at all when set to false. When set to 'on-demand' (the default
> -	value), fetch and pull will only recurse into a populated submodule
> +	recurse at all when set to false. When set to 'on-demand', 
> +	fetch and pull will only recurse into a populated submodule
> 	when its superproject retrieves a commit that updates the submodule's
> 	reference.
> +	If not set, it fallbacks to the value of `submodule.recurse`, and
> +	if both are not set the default value is 'on-demand’.

For such a change to the doc, I’d expect the commit message to point 
to the relevant part of the code that supports the behaviour that is being described, 
or to the specific test that enforces this behaviour (I briefly looked at t5526 but couldn’t spot one).
If such a test is missing it would be good to add it.

> 
> fetch.fsckObjects::
> 	If it is set to true, git-fetch-pack will check all fetched
> diff --git a/Documentation/config/push.txt b/Documentation/config/push.txt
> index 54871f8213..2a5250cf5c 100644
> --- a/Documentation/config/push.txt
> +++ b/Documentation/config/push.txt
> @@ -112,3 +112,6 @@ push.recurseSubmodules::
> 	is 'no' then default behavior of ignoring submodules when pushing
> 	is retained. You may override this configuration at time of push by
> 	specifying '--recurse-submodules=check|on-demand|no'.
> +	If not set, it fallbacks to the value of `submodule.recurse` (where
> +	a 'true' value means 'on-demand'), and if both are not set the
> +	default value is 'no’.

Same thing here. Also, I do not think it’s good UI that setting ‘submodule,recurse’ to true will make ‘push' default to ‘on-demand’ 
but will make ‘fetch’ unconditionally fetch, but I guess that would be for another series...

> -- 
> Patched on top of v2.25.1-377-g2d2118b814 (git version 2.25.1)
>