Alex Henrie wrote:

On Mon, Jun 21, 2021 at 4:12 PM Felipe Contreras
[off-list ref] wrote:

quoted

Alex Henrie wrote:

quoted

quoted

Although what needs to be done had been envisioned by some as early as
2013, the warning has only been around since Git 2.27 (released in
June 2020), and it was only restricted to pulls where fast-forwarding
is impossible in Git 2.31 (released in March 2021). The good news is
that (unless I'm mistaken) there are no more changes that need to be
made prior to changing the message from from "advise" to "die".

There is *a lot* that needs to be done.

 1. Update the documentation
 2. Add a --merge option (instead of the ackward --no-rebase)
 3. Fix all the wrong behavior with --ff, --no-ff, and -ff-only
 4. Add a pull.mode configuration
 5. Add a configuration for the mode in which we want to die
 6. Fix inconsistencies in the UI

I agree with you that the documentation should be updated when the
change is made (#1),

I'm saying the documentation needs to be updated _before_ the change is
made. There's no reason not to have the fast-forward example in the
documentation.

and maybe there should be a config option to go back to the behavior
of warning but doing the merge anyway (#5).

Before that we need a configuration to turn the behavior on.

The rest I think are things that would be nice to have but don't
preclude making the switch because aborting instead of merging would
not introduce any new UI limitations or inconsistencies.

They don't preclude the switch, but the switch should be precluded by a
warning, and the warning would be something like:

  The pull was not fast-forward, in the future you will have to choose a
  merge, or a rebase.

  To quell this message you have two main options:

  1. Adopt the new behavior:

    git config --global pull.mode ff-only

  2. Maintain the current behavior:

    git config --global pull.mode merge

  For now we will fall back to the traditional behavior (merge).

  Read "git pull --help" for more information.

Without having the changes I listed this warning can't be as useful.

Of course, it's ultimately up to Junio and the wider Git community,
and I would love to hear their thoughts about it.

I would not hold my breath.

quoted

In the meantime there's no reason to have subpar documentation.

My only serious objection to this patch is the instruction to merge if
you don't know what to do instead of asking the repository maintainer
what to do or reading the Git documentation. I don't have a strong
opinion on the rest of the patch.

I would be fine if the patch is merged without that line, but I believe
the patch is better with that line.

The line doesn't say "do this this if you don't know what to do", it
says if you are *unsure*. That is not the same thing.

And the user *already* did a merge, as that's what 'git pull' does by
default. The advice throws a warning, but proceeds with the merge.

The only thing the line is telling the user is how to muffle the message
if she is unsure of the previous muffling options.

Would you be happier with this?

  The simplest way to maintain the current behavior is to just do
  "git pull --no-rebase".

Cheers.

-- 
Felipe Contreras