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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Dec 7, 2020, 19:09 UTC
Message-ID: <CAMP44s2nmVnXiBA8S=vHBZznuRNKKe=xGOEBJ80MYhA_XCqNkg@mail.gmail.com>
In-Reply-To: <xmqqpn3lbhxn.fsf@gitster.c.googlers.com>

On Mon, Dec 7, 2020 at 12:16 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > When the user has specified "pull.mode=ff-only", the command dies with:
> >
> >   The pull was not fast-forward, please either merge or rebase.
> >
> > When the user has not specified any mode (by default), the command
> > would output a warning:
> >
> >   The pull was not fast-forward, in the future you will have to choose a
> >   merge, or a rebase.
>
> I quite don't get it.  They say the same thing.  At that point the
> user needs to choose between merge and rebase to move forward and
> ff-only would not be a sensible choice to offer the user.

The error happens *only* when the user has explicitly set "pull.mode=ff-only", and the pull dies. At that point; yes, the user must choose between one or the other.

But the warning happens by default, when the user has not chosen any mode, and the pull succeeds. The user doesn't need to choose; the pull continues as if the user chose the "merge" mode, which is the historic behavior.

They start by saying the same thing. But one errors out and says the user must choose, and the other warns that in the future the user must choose.

Show 12 quoted lines

> > Just to put this series in context: it's only part 1; it does not
> > introduce pull.mode, and it doesn't make --ff-only the default.
>
> I'd view the "in a non-fast-forward situation, the warning kicks in
> to those who haven't chosen between merge and rebase (i.e. no
> pull.rebase set to either true or false, and pull.ff not set to
> only), which is a bit more gentle than the current situtation" a
> good stopping point.  That state is already making ff-only the
> default for unconfigured users, or you can view it as shipping "git
> pull" in a shape that has the more dangerous half of its feature
> disabled to avoid hurting users.  So I am not sure why you keep
> saying you do not have --ff-only as the default.

The warning doesn't make the pull fail, ff-only does.

On the current master this works:

  test_expect_success 'non-fast-forward (default)' '
    setup_non_ff &&
    git pull
  '

And after my patch series (part 1) it still does. The default behavior has not changed.

I'm not sure what it is that I'm not explaining correctly.

-- 
Felipe Contreras