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
