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

Hi all,

Thank you all for the great feedback! I'm learning a lot as a
first-time contributor :) I will be wrapping my internship this week
and will continue contributing externally.

> >>
> >>>
> >>> 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.
>
> Mahi, since you're using Gitgitgadget, you would put this "since v1"
> content in the PR description, and Gitgitgadget will append it under
> the three-dash line when you /submit :) (Do keep the CC's automatically
> added by GGG so that your next version is CC'ed to those that participated
> in earlier rounds).
>

Got it, thank you!

> >
> > 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.
>
> I agree that we would not want yet another config variable that users would
> have to set. If people know about submodule.recurse and want to always use this
> behaviour, they already have it in their ~/.gitconfig, so they do not need a new
> variable. If they do not know about submodule.recurse, then they probably won't learn
> about this new variable either ;) That's why I suggested to Mahi that in any case it would
> be a good thing that 'git clone --recurse-submodules' would at least inform users, using
> an advice, that they might want to set submodule.recurse.
>

When discussing with the team, we revisited the feature.experimental
design. As we have yet to gain strong consensus on making this a
default config value, we've decided to ship it under a different
config value: submodule.stickyRecursiveClone. Now, if the user sets
submodule.stickyRecursiveClone=true, when they run git clone
--recurse-submodules, we will set submodule.recurse=true. While this
may mean a smaller dataset (only people who know of this flag), we can
still collect valuable data.

As for the advice message, I agree that would be a really useful
feature. I'll submit that as a different patch.

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

Unfortunately, the submodule.recurse variable can't be used as the
opt-in flag because this would cause commands to run recursively even
if developers don't have submodules in their project (aka don't run
git clone --recurse-submodules). That's why the alternate config value
seems a better choice at the moment.

Let me know what you guys think!

Best,
Mahi Kolla

  reply	other threads:[~2021-08-13  4:12 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
2021-08-13  3:35             ` Philippe Blain
2021-08-13  4:12               ` Mahi Kolla [this message]
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='CAN3QUFa3TRh9rvV3xS8XWd-dTvMqS=2j5Vhvp0wP33oPte9_2w@mail.gmail.com' \
    --to=mahikolla@google.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --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).