Re: Pull is Evil

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Matthieu Moy wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> ...
>> > Yes, this has been discussed many times in the past, and everyone agrees
>> > the default behavior is not correct.
>> 
>> You definitely have a strange notion of "everyone".
>
> Do I? Let's look at some of the discussions:
>
> http://thread.gmane.org/gmane.comp.version-control.git/225146
>
> * W. Trevor King agrees the default should change
> * Junio C Hamano agrees the default should change
> * John Keeping agrees the default should change
> * Matthieu Moy doesn't agree anything should change
> * Linus Torvalds agrees changing the default is fine
>
> http://thread.gmane.org/gmane.comp.version-control.git/233554
>
> * Richard Hansen agrees with my proposal
> * Ramkumar Ramachandra agrees with my proposal
> * Brian M. Carlson is not happy but can live with my proposal
> * Jeff King accepts my proposal is a good way to move forward
> * Matthieu Moy is OK with change, but only if the default remains the same
>
> So, by "everyone" I mean everyone but one person (you).

I looked at the latter thread and re-read what Peff wrote (added to
Cc).  I think the most relevant (other than solving it in quite a
different way $gmane/233554) one to your version of the solution is
this:

  http://thread.gmane.org/gmane.comp.version-control.git/233554/focus=234365

where he responds to my "how about this way forward" with this:

    > ... I think other people are also in
    > agreement. So perhaps:
    > 
    >  - drop jc/pull-training-wheel and revert its merge from 'next';
    > 
    >  - update Felipe's series with a bit of tweak to make it less
    >    impactful by demoting error into warning and advice.
    > 
    > would be a good way forward?

    I think that would address the concern I raised, because it does not
    create a roadblock to new users accomplishing their task. They can
    ignore the warning, or choose "merge" as the default to shut up the
    warning (and it is easy to choose that if you are confused, because
    it is what git is doing by default alongside the warning).

While I do not quite see the previous discussion as deciding the
particular implementation is good without further tweaks, I would
say that everybody agrees that the default behaviour is not good for
everybody and therefore should (or for Linus, "it is OK to") change.

> Rational people don't think in absolute terms, "everyone" means
> virtually everyone, which is the case.

True for "should change", not virtually everyone for "should change
with that particular solution".

But after re-reading the series description 0/n this round in the
other thread, I think the overall direction is good (just like Peff
said in the previous thread), especially if there is a warning not
error period.

The step (I am not sure you have it in your series or not, but I
would strongly recommend adding one if it doesn't yet) that gives a
"will change the default, and here is how to configure" warning when
we see an actual merge made (or rebased) after "git pull" without
"--merge/--rebase" is not just a way to prepare existing users, but
is a good way to bring new goodness to newbies.  The session might
go like this:

	$ git pull
        ... fetching ...
        ... merging ...
        ... diffstat ...
        warning: you merged the $branch from $remote into your
        warning: work, which may not be what you wanted to do unless
        warning: you are acting as a project integrator.  If that is
        warning: the case, "git config --set pull.mode ff-only" to
        warning: cause "git pull" to refuse working when it does not
        warning: fast-forward.  Use pull.mode=merge if you did mean
        warning: it, to squelch this message.

I am not advocating the exact wording above, but am illustrating
that there is a place for us to tell the new people to live in a
better future before the switchover happens.