Re: [PATCH v2 02/14] pull: improve default warning

From: Elijah Newren <newren@gmail.com>
Date: Dec 4, 2020, 22:59 UTC
Message-ID: <CABPp-BFdjj=+4jk0vo=kpNc6ug1=UgtKfXJZkseyyxut2VB=Uw@mail.gmail.com>
In-Reply-To: <20201204061623.1170745-3-felipe.contreras@gmail.com>

On Thu, Dec 3, 2020 at 10:16 PM Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 6 quoted lines

>
> We want to:
>
> 1. Be clear about what "specifying" means; merge or rebase.
> 2. Mention a direct shortcut for people that just want to get on with
>    their lives: git pull --no-rebase.

This is a shortcut for what?

Show 34 quoted lines

> 3. Have a quick reference for users to understand what this
>    "fast-forward" business means.
>
> This patch does all three.
>
> Thanks to the previous patch now "git pull --help" explains what a
> fast-forward is, and a further patch changes --no-rebase to --merge so
> it's actually user friendly.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin/pull.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 1034372f8b..22a9ffcade 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -346,17 +346,18 @@ static enum rebase_type config_get_rebase(void)
>
>         if (opt_verbosity >= 0 && !opt_ff) {
>                 advise(_("Pulling without specifying how to reconcile divergent branches is\n"
> -                        "discouraged. You can squelch this message by running one of the following\n"
> -                        "commands sometime before your next pull:\n"
> -                        "\n"
> -                        "  git config pull.rebase false  # merge (the default strategy)\n"
> -                        "  git config pull.rebase true   # rebase\n"
> -                        "  git config pull.ff only       # fast-forward only\n"
> -                        "\n"
> -                        "You can replace \"git config\" with \"git config --global\" to set a default\n"
> -                        "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
> -                        "or --ff-only on the command line to override the configured default per\n"
> -                        "invocation.\n"));
> +                       "discouraged; you need to specify if you want a merge, or a rebase.\n"

...want a merge, a rebase, or neither.

> +                       "You can squelch this message by running one of the following commands:\n"
> +                       "\n"
> +                       "  git config pull.rebase false  # merge (the default strategy)\n"

Should this be labelled as the default given the desire to make --ff-only the default? Perhaps I'm jumping ahead and you plan to change that later in this series.

Show 5 quoted lines

> +                       "  git config pull.rebase true   # rebase\n"
> +                       "  git config pull.ff only       # fast-forward only\n"
> +                       "\n"
> +                       "You can replace \"git config\" with \"git config --global\" to set a default\n"
> +                       "preference for all repositories.\n"

Good up to here.

> +                       "If unsure, run \"git pull --no-rebase\".\n"

Why is that safe to suggest? The original text may not have been the easiest to parse, but this seems more problematic to me.

> +                       "Read \"git pull --help\" for more information."

Nice addition.

Show 7 quoted lines

> +                       ));
>         }
>
>         return REBASE_FALSE;
> --
> 2.29.2
>