Thread (52 messages) 52 messages, 4 authors, 2020-12-07

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

From: Elijah Newren <hidden>
Date: 2020-12-05 00:57:08

Hi Felipe,

On Fri, Dec 4, 2020 at 4:12 PM Felipe Contreras
[off-list ref] wrote:
On Fri, Dec 4, 2020 at 5:00 PM Elijah Newren [off-list ref] wrote:
quoted
On Thu, Dec 3, 2020 at 10:16 PM Felipe Contreras
[off-list ref] wrote:
quoted
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?
  git config --global pull.rebase false
  git pull

It's a shorter way of saying: "do a 'git pull' like you've always done
but don't warn me".
quoted
quoted
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 <redacted>
---
 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.
There is no "git pull --no-merge". Years ago some people argued for a
"pull.mode=none" (essentially making "git pull" the same as "git
fetch"). But right now there's no option to do that.

There's an option to do --ff-only, but that's still a merge.
I disagree.  I'm well aware that checkout_fast_forward() (which is
what is ultimately called to do the fast-forwarding) is in a file
called merge.c, but that doesn't make it a merge.  I don't believe it
was anything more than a convenient place to dump some extra code at
the time.
Perhaps: a merge, a rebase, or a fast-forward?
Sure, that works; in fact, that's much better than my suggestion.  I like it.
quoted
quoted
+                       "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.
That's right.

In the previous series which does indeed make "pull.mode=ff-only" the
default [1], I do change the warning to specify the future default
[2], but in that series the warnings is changed to:

  The pull was not fast-forward, in the future you will have to choose
a merge, or a rebase.
  To squelch this message and maintain the current behavior, use:

    git config --global pull.mode merge

  To squelch this message and adopt the new behavior now, use:

    git config --global push.mode ff-only

  Falling back to old style for now (merge).
  Read "git pull --help" for more information.

Since that series didn't get any traction, I decided to only implement
step 1: fix the current situation. And later on another series would
do step 2: introduce "pull.mode=ff-only" and do the preparations to
make it the default.
I like this longer plan.  However, on the shorter scale plan...I think
the suggestion to use "git pull --no-rebase" makes the current
situation worse rather than fixing it.
quoted
quoted
+                       "  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.
quoted
+                       "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.
Because "git pull" has been doing the same as "git pull --no-rebase"
for more than a decade. It's safe for people to continue with this
behavior for a few more months.

Some people need to get things done today, and they are not interested
in future changes, nor changing their default configuration, or what
the warning has to say.

They just want "git pull" to do the same as yesterday, and the year
before, without being bothered with an annoying warning.

Those people can start training their fingers to do "git pull
--merge", and learn the problems with "git pull" later.

We want to respect the user's time, and not force them to read the
warning today.
The warning was added because sending users down paths that break
things badly is a waste of user's time, and often a much bigger waste
of user's time than making them read up on the meaning behind the two
different choices of what kind of changes they can make.  I agree the
warning went too far, but I fully agree with the original folks who
put this warning here that we need to have it for at least some cases
and that there is a decision to be made.  Though I am just one voice,
and perhaps others will agree with you on your point here, I'll
continue to disagree with blindly suggesting "git pull --no-rebase".
quoted
quoted
+                       "Read \"git pull --help\" for more information."
Nice addition.
Especially since now it explains what a fast-forward is.
Indeed.  :-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help