Re: [PATCH 0/3] Reject non-ff pulls by default

From: Jeff King <peff@peff.net>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: John Keeping <john@keeping.me.uk>,
	Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Andreas Krey <a.krey@gmx.de>
Subject: Re: [PATCH 0/3] Reject non-ff pulls by default
Date: Sun, 8 Sep 2013 03:50:46 -0400	[thread overview]
Message-ID: <20130908075046.GL14019@sigill.intra.peff.net> (raw)
In-Reply-To: <CAMP44s3LLHL=oP2PFr4b7VD0dL4yGBOL00O_GWj8eZLrYNM3kg@mail.gmail.com>

On Sun, Sep 08, 2013 at 02:15:17AM -0500, Felipe Contreras wrote:

> > I think "the guy" may be git itself. For example, here is a possible
> > session with jc/pull-training-wheel:
> >
> >   $ git push
> 
> Who told him to use 'git push'? Certainly not git.

Any of the hundreds of existing tutorials that teach basic git commands
like "push"?

> >   To ...
> >    ! [rejected]        master -> master (non-fast-forward)
> >   error: failed to push some refs to '...'
> >   hint: Updates were rejected because the tip of your current branch is behind
> >   hint: its remote counterpart. Integrate the remote changes (e.g.
> >   hint: 'git pull ...') before pushing again.
> >   hint: See the 'Note about fast-forwards' in 'git push --help' for details.
> [...]
> 
> Why stop there? Post the whole man page already.
> 
> Moreover, it's overly verbose on all the wrong and irrelevant
> information. If you are going to waste precious screen state, explain
> wth a "non fast-forward" is; people can figure out what a merge is,
> and maybe a rebase, but a "non fast-forward" definitely not.

Note that I was not trying to defend any of the messages, but only
showing a plausible mechanism by which a user with basic knowledge that
he wants to push may arrive at the question "what is the difference
between merge and rebase?".

If you want to suggest revisions for the push message, go ahead. The
push advice _is_ an attempt to define non-fast-forwards in plain
language without taking up too much space, but perhaps you can do
better. You could even suggest omitting it entirely, but I'm not sure if
that is a good idea. It was not added in a vacuum; we lacked that advice
for many years, and people complained about it quite a bit until it was
added.

> > The user is pointed at "pull" from "push", and then gets presented with
> > the "merge or rebase" choice. It may be that the advice you can find by
> > googling "merge vs rebase" is enough to then help the person along
> > (and/or we may need to improve the manpages in that respect).
> 
> Yes, but that's not the use-case we are talking about. You mentioned
> specifically a "svn-like" worfklow where the guy was told by somebody
> else to replace the svn commands with git ones.

No, I mentioned an "svn-like" workflow. I didn't say anything about how
they were told. They might have been told by a co-worker, or read a
brief tutorial on git, or read something like "Git-SVN Crash Course".

> If we are talking about a guy that is learning git, that's and
> entirely different case.

That is certainly what I meant to be talking about.

> The purpose of this change in the code is not to change the user
> behavior. The choice of merge vs. rebase is entirely up to the user,
> and we are not changing that.

Right, but by not doing anything by default, you are forcing the user to
make a decision. Right now, we strongly encourage merging by making it
the default, and you have to learn about rebasing separately. But a
message that mentions them both as equals is going to lead to extra work
for the user; they have to figure out which one is most appropriate. My
concern is that this is non-trivial for new users, and that they may end
up arbitrarily picking rebase, which is probably not doing them any
favors if they do not understand it.

For clueful users, choosing between the two is not hard. But some people
seem to have trouble understanding the DAG. I don't know how large a
group that is, and how any pain caused by this change might compare to
the times it will help.

> > If you are interested, I can ask the opinion of some of the GitHub
> > trainers. They see a lot of new users and have a sense of what kinds of
> > confusion come up most frequently, what kinds of workflows they tend to
> > see, etc. Their experience may be biased towards corporate-ish users,
> > though, because those are the people who pay for training.
> 
> Ask. I'm sure they will tell you doing merges by mistake with 'git
> pull' is an issue.

I've sent an email. I'll post the response when I get it.

> >> "Any more babysitting with essay long messages is counter-productive
> >> to the vast majority of Git users."
> >
> > I think that is what we have advice.* for.
> 
> I don't understand what that means.

It means that some time ago, after many people complained that git did
not give enough hints, we added many hints. Some people who did not need
these hints would want to disable them, and we have the "advice.*"
config options to do so. So we can have a longer message for new users,
and a shorter one for people who do not want to be bothered with the
long advice.

-Peff