Re: [PATCHv2] push: change submodule default to check

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> When working with submodules, it is easy to forget to push the submodules.
> The setting 'check', which checks if any existing submodule is present on
> at least one remote of the submodule remotes, is designed to prevent this
> mistake.
>
> Flipping the default to check for submodules is safer than the current
> default of ignoring submodules while pushing.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> Slightly reworded commit message than in v1,
> (https://public-inbox.org/git/20160817204848.8983-1-sbeller@google.com/)
> The patch itself is however the same.
>
> I just push it out now with a new commit message, such that we can easier
> pick it up later for Git 3.0, when changes that change default make more sense.
>
> As said in an earlier message, you could however also argue that this is
> fixing a bug in your workflow, so it might be worth fixing before 3.0
> as well. I dunno.

With this change suddenly landing on the version of Git a user just
updated, the only possible negative effect would be that somebody
who did not configure push.recurseSubmodules suddenly starts getting
an error.  What would the error message say?

What I want you to think about and explain in the justification is
if it gives the user enough information to decide the best course of
action to adjust to the new world order, when the user's expectation
has been that the superproject gets pushed independent from its
submodules.  If the existing error message gives enough information,
explains why Git suddenly started refusing the push is generally a
good thing for the user, tells some minority users (in whose
workflow it is perfectly safe to push out the toplevel independently
from the submodule) how to turn it back to the old behaviour clearly
enough, then this single-liner may be good enough.  Otherwise we may
need a new logic to allow us to see if recurse_submodules that is
set to RECURSE_SUBMODULES_CHECK is because the user explicitly
configured, or because the user did not have any configuration, and
issue a different message in the latter case.



>  builtin/push.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 3bb9d6b..479150a 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -22,7 +22,7 @@ static int deleterefs;
>  static const char *receivepack;
>  static int verbosity;
>  static int progress = -1;
> -static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> +static int recurse_submodules = RECURSE_SUBMODULES_CHECK;
>  static enum transport_family family;
>  
>  static struct push_cas_option cas;