git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Emily Shaffer <emilyshaffer@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Mahi Kolla via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Mahi Kolla <mahikolla@google.com>
Subject: Re: [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag
Date: Thu, 12 Aug 2021 16:54:12 -0700	[thread overview]
Message-ID: <YRW0pGXXWnY7C470@google.com> (raw)
In-Reply-To: <xmqqeeaz70ph.fsf@gitster.g>

On Wed, Aug 11, 2021 at 09:20:58PM -0700, Junio C Hamano wrote:
> 
> "Mahi Kolla via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > From: Mahi Kolla <mahikolla@google.com>
> >
> > Currently, when running 'git clone --recurse-submodules', developers do not expect other commands such as 'pull' or 'checkout' to run recursively into active submodules. However, setting 'submodule.recurse' to true at this step could make for a simpler workflow by eliminating the '--recurse-submodules' option in subsequent commands. To collect more data on developers' preference in regards to making 'submodule.recurse=true' a default config value in the future, deploy this feature under the opt in feature.experimental flag.
> 
> Please wrap overlong lines in your proposed log message to say 70 or
> so columns.
> 
> >
> > Since V1: Made this an opt in feature under the experimental flag. Updated tests to reflect this design change. Also updated commit message.
> 
> This does not belong to the commit log message proper.  Noting the
> difference between the version being submitted and the pervious one
> this way is a way to help reviewers and is very much appreciated,
> but please do so below the three-dash line below your sign-off.
> 
> > Signed-off-by: Mahi Kolla <mahikolla@google.com>
> > ---
> >     clone: set submodule.recurse=true if feature.experimental flag enabled
> 
> The proposed approach misuses feature.experimental flag, which was
> designed to turn on many new features at once.  The features covered
> by the flag share one common trait: they all have gained consensus
> that in the longer term we would hopefully be able to make it on by
> default, and give early adopters an easy way to turn them all on.
> 
> I do not think setting submodule.recurse=true upon "clone --recurse"
> falls into that category just yet.  If we were to make this opt-in,
> we'd want a separate flag, so that those early adopters who are
> dogfooding other features that have consensus that they are
> hopefully the way of the future won't have to be forced into this
> separate feature.

I'd like to open discussions to get said consensus :)

It seems surprising to me that a user would want to clone with all the
submodules fetched *without* intending to then use
superproject-plus-submodules together recursively. I would like to hear
more about the use case you have in mind, Junio.

One scenario that did come to mind when I discussed this with Mahi is
that a user may provide a pathspec to --recurse-submodules (that is,
"yes, this repo has submodules a/ and b/, but I only care about the
contents of submodule a/") - and in that case, --recurse-submodules
seems to do the right thing with or without Mahi's change.

It seemed to me that trying out this change on feature.experimental flag
was the right approach, because users with that flag have already
volunteered to be testers for upcoming behavior changes; this seems like
one such that is likely to be welcome. By contrast, turning the behavior
on with a separate config variable reduces the pool of testers
essentially to "users who know about this change" - or, to be more
reductive, "a handful of users at Google who we Google Git contributors
already know want this change". I recommended to Mahi that we stick this
feature under 'feature.experimental' because I really wanted to hear
from more users than just Googlers.

> 
> Perhaps a separate (and new) configuration variable (in ~/.gitconfig
> perhaps) can be used as that opt-in flag (I wonder if the existing
> submodule.recurse variable can be that opt-in flag, though).
> 

Do you mean something like "git config --global submodule.recurse
TryTheNewThingPlease"? I guess it could work - repos that use a pathspec
in that slot would still have the pathspec configured locally, repos
that have submodule.recurse intentionally unset wouldn't know what to do
with the junk string, and repos that have submodule.recurse
intentionally set to true would still have that true override the global
value.

Or else I misunderstood you...

 - Emily

  reply	other threads:[~2021-08-12 23:54 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02 17:29 [PATCH 0/2] clone: update submodule.recurse in config when using --recurse-submodule Mahi Kolla via GitGitGadget
2021-08-02 17:29 ` [PATCH 1/2] " Mahi Kolla via GitGitGadget
2021-08-02 17:29 ` [PATCH 2/2] " Mahi Kolla via GitGitGadget
2021-08-02 22:38 ` [PATCH v2 0/3] " Mahi Kolla via GitGitGadget
2021-08-02 22:38   ` [PATCH v2 1/3] " Mahi Kolla via GitGitGadget
2021-08-02 22:38   ` [PATCH v2 2/3] " Mahi Kolla via GitGitGadget
2021-08-02 22:38   ` [PATCH v2 3/3] clone test: update whitespace according to style guide Mahi Kolla via GitGitGadget
2021-08-02 23:23   ` [PATCH v3 0/4] clone: update submodule.recurse in config when using --recurse-submodule Mahi Kolla via GitGitGadget
2021-08-02 23:23     ` [PATCH v3 1/4] " Mahi Kolla via GitGitGadget
2021-08-03  3:20       ` Philippe Blain
2021-08-07  3:06         ` Mahi Kolla
2021-08-02 23:23     ` [PATCH v3 2/4] " Mahi Kolla via GitGitGadget
2021-08-02 23:23     ` [PATCH v3 3/4] clone test: update whitespace according to style guide Mahi Kolla via GitGitGadget
2021-08-02 23:23     ` [PATCH v3 4/4] clone: " Mahi Kolla via GitGitGadget
2021-08-03  3:08     ` [PATCH v3 0/4] clone: update submodule.recurse in config when using --recurse-submodule Philippe Blain
2021-08-03 22:41     ` Junio C Hamano
2021-08-09 19:11     ` [PATCH v4] " Mahi Kolla via GitGitGadget
2021-08-09 21:15       ` Junio C Hamano
2021-08-10  7:26         ` Mahi Kolla
2021-08-10 18:36           ` Junio C Hamano
2021-08-10 23:04             ` Philippe Blain
2021-08-10 23:59               ` Mahi Kolla
2021-08-11  5:02             ` Junio C Hamano
2021-08-12  2:46       ` [PATCH v5] clone: set submodule.recurse=true if user enables feature.experimental flag Mahi Kolla via GitGitGadget
2021-08-12  4:20         ` Junio C Hamano
2021-08-12 23:54           ` Emily Shaffer [this message]
2021-08-13  3:35             ` Philippe Blain
2021-08-13  4:12               ` Mahi Kolla
2021-08-13 19:53                 ` Junio C Hamano
2021-08-13 14:59               ` Junio C Hamano
2021-08-13  4:34             ` Junio C Hamano
2021-08-13 20:23               ` Emily Shaffer
2021-08-13 20:30                 ` Junio C Hamano
2021-08-13 20:38                   ` Emily Shaffer
2021-08-13 20:48                     ` Mahi Kolla
2021-08-13 20:52                 ` Junio C Hamano
2021-08-13 17:33         ` Felipe Contreras
2021-08-14  1:09         ` [PATCH v6] clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled Mahi Kolla via GitGitGadget
2021-08-14 18:05           ` Junio C Hamano
2021-08-17 23:02             ` Emily Shaffer
2021-08-18 19:57               ` Junio C Hamano
2021-08-18 20:15                 ` [PATCH] fixup! " Junio C Hamano
2021-08-19 17:41                   ` Emily Shaffer
2021-08-30 20:59                     ` Emily Shaffer
2021-08-30 21:23                       ` Junio C Hamano
2021-08-18 22:40           ` [PATCH v6] " Philippe Blain

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=YRW0pGXXWnY7C470@google.com \
    --to=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=levraiphilippeblain@gmail.com \
    --cc=mahikolla@google.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).