git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Glen Choo <chooglen@google.com>
To: Philippe Blain <levraiphilippeblain@gmail.com>, git@vger.kernel.org
Subject: Re: [RFC PATCH 2/2] branch: add --recurse-submodules option for branch creation
Date: Wed, 22 Sep 2021 10:24:50 -0700	[thread overview]
Message-ID: <kl6lsfxw34m5.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <084caed9-5854-9fe2-8476-b5325ddac52c@gmail.com>

Philippe Blain <levraiphilippeblain@gmail.com> writes:

>> 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.

Ah, thanks! I really appreciate the style pointers.

> 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'.

That's a good point. This behavior of git branch --recurse-submodules
has the potential to directly conflict with established workflows.
Opting out via 'branch.recurseSubmodules seems reasonable and I think we
definitely want to give users that fine-grained control. I wonder
if we should take this even further - perhaps --recurse-submodules is
too disruptive to be controlled by the config value as checkout, fetch,
pull, etc.

Perhaps submodule.recurse should be more fine-grained, so we can ship
good defaults for different workflows e.g. options that map to the
current true/false behavior and another option that includes branching
(I wish I could give these names, but I have no clue what to name them).

> 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.

I think that's a good call. It also looks like the tests in this file
are somewhat old, so it would be nice to start on a clean slate :)

>> In this patchset, branching works slightly differently between the
>> superproject and submodule (skip ahead for specifics). There are two
>> very obvious alternatives that can address this:
 
>> * A: only implement --recurse-submodules behavior after we are able to
>>   eliminate any kind of dependence on the_repository/global state that
>>   shouldn't be shared.
>> * B: implement --recurse-submodules as child processes, which won't be
>>   bothered by global state.

Your comments on the UX make a lot of sense, and I'll think about that
more carefully. Let me know if you also have thoughts on the
implementation itself, particular on in-process vs child process :)

      reply	other threads:[~2021-09-22 17:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 23:25 [RFC PATCH 0/2] branch: implement in-process --recurse-submodules Glen Choo
2021-09-21 23:25 ` [RFC PATCH 1/2] refs: pass struct repository *r through to write_ref_to_lockfile() Glen Choo
2021-09-21 23:25 ` [RFC PATCH 2/2] branch: add --recurse-submodules option for branch creation Glen Choo
2021-09-22 11:10   ` Ævar Arnfjörð Bjarmason
2021-09-22 16:55     ` Glen Choo
2021-09-22 12:28   ` Philippe Blain
2021-09-22 17:24     ` Glen Choo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=kl6lsfxw34m5.fsf@chooglen-macbookpro.roam.corp.google.com \
    --to=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).