From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Wed, 22 Sep 2021 12:28:57 GMT
Subject: Re: [RFC PATCH 2/2] branch: add --recurse-submodules option for branch creation
Message-ID: <084caed9-5854-9fe2-8476-b5325ddac52c@gmail.com>
In-Reply-To: <20210921232529.81811-3-chooglen@google.com>

Hi Glen,

Le 2021-09-21 à 19:25, Glen Choo a écrit :
> When working with a superproject and submodules, it can be helpful to
> create topic branches to coordinate the work between each repository.
> 
> Teach cmd_branch to accept the --recurse-submodules option when creating
> branches. When specified, like
> 
>    git branch --recurse-submodules topic
> 
> git-branch will create the "topic" branch in the superproject and all
> submodules.
> 
> recurse_submodules is only supported for creating a branch. git-branch

small nit: we usually don't refer to the dashed form of commands anymore: 'git branch'
would be preferable.

> will fail if the user attempts to use --recurse-submodules with anything
> other than creating a branch. If a user has submodule.recurse in their
> config, git-branch will use --recurse-submodules for creating a branch,
> but will treat it as unset for any other operation.

That seems like a good starting point.

However, I think that for this new feature, I would prefer *also* having
a new config 'branch.recurseSubmodules' (or similar) that would allow
more granular control than 'submodule.recurse', which influences several
commands. Personnally I have 'submodule.recurse' set to true in my '~/.gitconfig',
because I want the submodules working trees to be updated when I use 'git checkout'
to change branches, 'git grep' to search the working tree, etc., but I usually
do not *work* on the submodules in my project and I would not like new branches
being created in them every time I create a new branch in the superproject.
In other words, 'branch.recurseSubmodules=false' would have higher priority
than 'submodule.recurse=true'.

> 
> There is no easy way to get the remotes of a submodule because remotes.c
> stores state as static variables. As such, branch tracking is disabled
> when using --recurse-submodules.
> 
> Signed-off-by: Glen Choo <chooglen@google.com>
> ---


> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index e575ffb4ff..858831d4cf 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1421,5 +1421,63 @@ test_expect_success 'invalid sort parameter in configuration' '
>   		test_must_fail git branch
>   	)
>   '
> +# Submodule tests
> +
> 

Most tests for submodules are usually in separate test files. I don't think
this is a set-in-stone rule, but if more tests are coming in the future, maybe
a new test file t????-branch-submodule.sh would be appropriate ? Just a small suggestion.


Thanks for working on this! It's exciting to see new developments
in the submodule area :)

Philippe.