git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Jeff King <peff@peff.net>
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 02:15:17 -0500	[thread overview]
Message-ID: <CAMP44s3LLHL=oP2PFr4b7VD0dL4yGBOL00O_GWj8eZLrYNM3kg@mail.gmail.com> (raw)
In-Reply-To: <20130908065420.GI14019@sigill.intra.peff.net>

On Sun, Sep 8, 2013 at 1:54 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Sep 08, 2013 at 01:17:42AM -0500, Felipe Contreras wrote:
>
>> > I think it's fine to tell them to do "git pull --merge". What I'd worry
>> > more about is somebody who is suddenly presented with the choice between
>> > "--rebase" and "--merge" and doesn't know which to choose. We've created a
>> > cognitive load on the user, and even more load if they choose --rebase
>> > and don't quite understand what it means.
>>
>> If that happens they will go back to the guy that told them to run
>> those commands.
>
> 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.

>   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.
>
>   $ git pull

>   The pull does not fast-forward; please specify
>   if you want to merge or rebase.
>
>   Use either
>
>       git pull --rebase
>       git pull --merge
>
>   You can also use 'git config pull.rebase true' (if you want --rebase) or
>   'git config pull.rebase false' (if you want --merge) to set this once for
>   this project and forget about it.

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.

> 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.

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

> I am genuinely curious what people in favor of this feature would want
> to say in the documentation to a user encountering this choice for the
> first time. In my experience, rebasing introduces more complications,
> specifically:

Yes, but it's what the user might want.

> I know those are all balanced by some advantages of rebasing, but I also
> think they are things that can be troublesome for a user who does not
> fully grok the rebase process. I'm just wondering if we should mention
> both, but steer people towards merging as the safer alternative (you
> might have ugly history, but you are less likely to create a mess with
> duplicate commits or badly-resolved conflicts).

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.

The purpose of this change is to avoid doing a merge when the user
wanted a rebase, or maybe something more complicated. So a rebase
being complicated is not an issue, because we know that's what the
user wants, that's the whole reason we are trying to avoid the
automated merge.

>> Fortunately there probably are very few of these users.
>
> Maybe. I am not sure how one would measure.
>
> 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.

>> > The current warning message in jc/pull-training-wheel is quite neutral
>> > between the two options. Perhaps we should lean more towards merging?
>>
>> I don't like that message. I would like this for the deprecation period:
>>
>> "The pull was not fast-forward, in the future you would have to choose
>> a merge or a rebase, merging automatically for now. Read 'man git
>> pull' for more help."
>>
>> Then when obsolete:
>>
>> The pull was not fast-forward, please either merge or rebase.
>
> A deprecation message helps people who are making the transition from an
> older behavior to a newer one. It cannot help new users who start with a
> git version after the deprecation period.

The new users are told to either merge or rebase, if they don't know
what that means, they will go on look it up, just like they looked up
the 'git pull' command in the first place.

>> "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.

-- 
Felipe Contreras

  reply	other threads:[~2013-09-08  7:15 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-31 22:38 [PATCH 0/3] Reject non-ff pulls by default Felipe Contreras
2013-08-31 22:38 ` [PATCH 1/3] merge: simplify ff-only option Felipe Contreras
2013-08-31 22:38 ` [PATCH 2/3] t: replace pulls with merges Felipe Contreras
2013-08-31 22:38 ` [PATCH 3/3] pull: reject non-ff pulls by default Felipe Contreras
2013-09-03 17:21 ` [PATCH 0/3] Reject " Junio C Hamano
2013-09-03 21:50   ` Felipe Contreras
2013-09-03 22:38     ` Junio C Hamano
2013-09-03 22:59       ` Felipe Contreras
2013-09-04  8:10       ` John Keeping
2013-09-04  9:25         ` Jeff King
2013-09-04 10:16           ` John Keeping
2013-09-08  2:52           ` Felipe Contreras
2013-09-08  4:18             ` Jeff King
2013-09-08  4:37               ` Felipe Contreras
2013-09-08  4:43                 ` Jeff King
2013-09-08  5:09                   ` Felipe Contreras
2013-09-08  5:21                     ` Jeff King
2013-09-08  6:17                       ` Felipe Contreras
2013-09-08  6:54                         ` Jeff King
2013-09-08  7:15                           ` Felipe Contreras [this message]
2013-09-08  7:50                             ` Jeff King
2013-09-08  8:43                               ` Felipe Contreras
2013-09-09 20:17                               ` Jeff King
2013-09-09 22:59                                 ` Felipe Contreras
2013-09-08 10:03                           ` John Keeping
2013-09-09 20:04                             ` Jeff King
2013-09-08 17:26                 ` brian m. carlson
2013-09-08 22:38                   ` Felipe Contreras
2013-09-09  0:01                     ` brian m. carlson
2013-09-09  0:29                       ` Felipe Contreras
2013-09-09  0:36                         ` Felipe Contreras
2013-09-09  0:38                           ` brian m. carlson
2013-09-09  7:18                         ` Matthieu Moy
2013-09-09 18:47                           ` Junio C Hamano
2013-09-09 19:52                             ` Jeff King
2013-09-09 20:24                               ` John Keeping
2013-09-09 20:44                                 ` Jeff King
2013-09-09 21:10                                   ` John Keeping
2013-09-09 21:48                                   ` Richard Hansen
2013-09-09 20:50                                 ` Matthieu Moy
2013-09-09 20:53                                   ` Jeff King
2013-09-09 21:34                                     ` Philip Oakley
2013-09-09 23:02                                 ` Felipe Contreras
2013-09-10  8:08                                   ` John Keeping
2013-09-09 20:47                             ` Matthieu Moy
2013-09-10 21:56                               ` Junio C Hamano
2013-09-09 23:17                           ` Felipe Contreras
2013-09-10  8:26                             ` Matthieu Moy
2013-09-11 10:53                               ` Felipe Contreras
2013-09-11 11:38                                 ` Matthieu Moy
2013-09-13  0:55                                   ` Felipe Contreras
2013-09-04 16:59         ` Junio C Hamano
2013-09-04 17:17         ` Junio C Hamano
2013-09-04 22:08           ` Philip Oakley
2013-09-04 22:59             ` Junio C Hamano
2013-09-05  8:06               ` John Keeping
2013-09-05 19:18                 ` Junio C Hamano
2013-09-05 19:26                   ` John Keeping
2013-09-06 21:41                     ` Jonathan Nieder
2013-09-06 22:14                       ` Junio C Hamano
2013-09-07 11:07                         ` John Keeping
2013-09-08  2:36                         ` Felipe Contreras
2013-09-08  2:34                 ` Felipe Contreras
2013-09-08  8:01                   ` Philip Oakley
2013-09-08  8:16                     ` Felipe Contreras
2013-09-08  8:42                       ` Philip Oakley
2013-09-08  8:49                         ` Felipe Contreras
2013-09-08 10:02                           ` Philip Oakley
2013-09-08 10:39                             ` Philip Oakley
2013-09-05 11:01               ` John Szakmeister
2013-09-05 11:38                 ` John Keeping
2013-09-05 12:37                   ` John Szakmeister
2013-09-05 15:20               ` Richard Hansen
2013-09-05 21:30               ` Philip Oakley
2013-09-05 23:45                 ` Junio C Hamano
2013-09-05 23:38               ` Junio C Hamano
2013-09-08  2:41               ` Felipe Contreras
2013-09-08  6:17                 ` Richard Hansen
2013-09-08 18:10                   ` Junio C Hamano
2013-09-08 20:05                     ` Richard Hansen
2013-09-08 22:46                     ` Philip Oakley
2013-09-08 22:46                     ` Felipe Contreras
2013-09-08 23:11                       ` Ramkumar Ramachandra
2013-09-05 13:31           ` Greg Troxel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMP44s3LLHL=oP2PFr4b7VD0dL4yGBOL00O_GWj8eZLrYNM3kg@mail.gmail.com' \
    --to=felipe.contreras@gmail.com \
    --cc=a.krey@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=john@keeping.me.uk \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).