All of lore.kernel.org
 help / color / mirror / 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: Fri, 13 Aug 2021 13:23:46 -0700	[thread overview]
Message-ID: <YRbU0uBxqinQ1EnE@google.com> (raw)
In-Reply-To: <xmqqpmuiynbs.fsf@gitster.g>

On Thu, Aug 12, 2021 at 09:34:47PM -0700, Junio C Hamano wrote:
> 
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > 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.
> 
> You may need full forest of submodules with the superproject to
> build your ware (i.e. you'd probably want to clone and fetch-update
> them), but you may only be working on the sources in a small subset
> of submodules and do not need your recursive grep or diff to go
> outside that subset, for example.  You'd need to ask the people who
> recursively clone and not set submodule.recurse to true (I am not
> among them).
> 
> > 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.
> 
> Please be a bit more specific about "the right thing".  Do you mean
> "the submodules that matched the pathspec gets recursed into by
> later operations"?
> 
> If so, "git clone --resurse-submodules=. $from_there" may perhaps be
> the "there is no way to we make this opt-in?" I have been asking
> about (not "asking for")?
> 
> > 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
> 
> Yes, if we already have a consensus that a proposed change is
> something we hope to be desirable, then feature.experimental is a
> good way to see if early adopters can find problems in their real
> world use, as these volunteers may include audiences with different
> use pattern from the original advocates of a particular feature, who
> might have dogfooded the new feature to gain consensus that it may
> want to become the default.
> 
> By the way, I am not fundamentally opposed to the feature being
> proposed.  I would imagine that such a feature would be liked by
> those who want to keep things simpler.  I however am hesitant to see
> it pushed too hastily without considering if it harms existing users
> with different preferences.
> 
> IOW, I was primarily reacting to the apparent wrong order in which
> things are being done, first throwing this into feature.experimental
> before we have gathered enough confidence that it may be a good
> thing to do by having it in shipped version as an opt-in feature.

Yeah, since writing my reply I was very helpfully reinformed on the
convention around 'feature.experimental' by Jonathan N off-list. Thanks
for being patient with me.

I think the right move, then, is to explore whether your suggestion in
https://lore.kernel.org/git/xmqqeeaxw28z.fsf%40gitster.g is appropriate
- I have the sense that it is, but I want to make sure to think it
through before I say so for sure. 

 - Emily

  reply	other threads:[~2021-08-13 20:23 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
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 [this message]
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

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

  git send-email \
    --in-reply-to=YRbU0uBxqinQ1EnE@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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.