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 18:43:15

On Fri, Dec 4, 2020 at 5:56 PM Felipe Contreras
[off-list ref] wrote:
On Fri, Dec 4, 2020 at 6:56 PM Elijah Newren [off-list ref] wrote:
quoted
Hi Felipe,

On Fri, Dec 4, 2020 at 4:12 PM Felipe Contreras
[off-list ref] wrote:
quoted
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
quoted
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.
Yeah. It's a better plan, just more work for me.
quoted
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.
Well, I already said I partly agree with you: in the --ff-only case
the suggestion should not be brought forward.

But in the "git pull" default case, *today* it's doing a merge. If
uttering --merge and thus making the current behavior explicit instead
of implicit seems dangerous it's because it is. But not documenting it
doesn't make it any less dangerous.
Sounds like we agree that the future should be ff-only-as-default.  I
also agree with you that the primary problem is the current default
behavior, and I'll agree with you that documenting the current default
is okay.  However, I disagree that your wording here:

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

does anything of the sort.  It does not mention that this is the
default behavior the users would get if they provided no flags.  More
importantly, it makes a recommendation...and one that undercuts the
point of the message.  It makes it feel like the message shouldn't
exist at all in any circumstances.  I even suspect that adding that
sentence may undercut any efforts towards changing the default to
ff-only-as-default.  While I'm a big fan of most of what you've done
in this series, I will object to its merging for as long as this
stays.  (I definitely don't have veto power or anything close to it,
just stating what my opinion is.)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help