All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Hansen <rhansen@bbn.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Philip Oakley <philipoakley@iee.org>,
	John Keeping <john@keeping.me.uk>,
	Felipe Contreras <felipe.contreras@gmail.com>,
	git@vger.kernel.org, Andreas Krey <a.krey@gmx.de>
Subject: Re: [PATCH 0/3] Reject non-ff pulls by default
Date: Thu, 05 Sep 2013 11:20:43 -0400	[thread overview]
Message-ID: <5228A14B.3000804@bbn.com> (raw)
In-Reply-To: <xmqqr4d4jird.fsf@gitster.dls.corp.google.com>

On 2013-09-04 18:59, Junio C Hamano wrote:
> "Philip Oakley" <philipoakley@iee.org> writes:
> 
>> From: "Junio C Hamano" <gitster@pobox.com>
>>> John Keeping <john@keeping.me.uk> writes:
>>>
>>>> I think there are two distinct uses for pull, which boil down to:
>>>>
>>>>     (1) git pull
>>> ...
>>> Peff already covered (1)---it is highly doubtful that a merge is
>>> "almost always wrong".  In fact, if that _were_ the case, we should
>>> simply be defaulting to rebase, not failing the command and asking
>>> between merge and rebase like jc/pull-training-wheel topic did.
>>>
>>> We simply do not know what the user wants, as it heavily depends on
>>> the project, so we ask the user to choose one (and stick to it).
>>
>> We only offer a limited list. It won't be sufficient for all use
>> cases. It wasn't for me.
> 
> Very interesting. Tell us more.

I'm a bit late to the discussion, but I wanted to chime in.  I detest
'git pull' and discourage everyone I meet from using it.  See:
<http://stackoverflow.com/questions/15316601/why-is-git-pull-considered-harmful>
for my reasons.

Instead, I encourage people to do this:

   git config --global alias.up '!git remote update -p; git merge
--ff-only @{u}'

and tell them to run 'git up' whenever they would be tempted to use a
plain 'git pull'.

I usually work with a central repository with topic branches.  I follow
this rule of thumb:
  * When merging a "same-named" branch (e.g., origin/foo into foo, foo
    into origin/foo), it should always be a fast-forward.  This may
    require rebasing.
  * When merging a "differently-named" branch (e.g., feature.xyz into
    master), it should never be a fast-forward.

In distributed workflows, I think of 'git pull <collaborator-repo>
<their-branch>' as merging a differently-named branch (I wouldn't be
merging if they hadn't told me that a separate feature they were working
on is complete), so I generally want the merge commit.  But when I do a
'git pull' without extra arguments, I'm updating a same-named branch so
I never want a merge.

When merging a differently-named branch, I prefer the merge --no-ff to
be preceded by a rebase to get a nice, pretty graph:

       * merge feature.xyz  <- master
       |\
       | * xyz part 3/3
       | * xyz part 2/3
       | * xyz part 1/3
       |/
       * merge feature.foo
       |\
       | * foo part 2/2
       | * foo part 1/2
       |/
       * merge feature.bar
       |\
       ...

The explicit merge has several benefits:
  * It clearly communicates to others that the feature is done.
  * It makes it easier to revert the entire feature by reverting the
    merge if necessary.
  * It allows our continuous integration tool to skip over the
    work-in-progress commits and test only complete features.
  * It makes it easier to review the entire feature in one diff.
  * 'git log --first-parent' shows a high-level summary of the changes
    over time, while a normal 'git log' shows the details.

> 
> When "git pull" stops because what was fetched in FETCH_HEAD does
> not fast-forward, then what did _you_ do (and with the knowledge you
> currently have, what would you do)?

I stop and review what's going on, then make a decision:
  * usually it's a rebase
  * sometimes it's a rebase --onto (because the branch was
    force-updated to undo a particularly bad commit)
  * sometimes it's a rebase -p (because there's an explicit merge of a
    different branch that I want to keep)
  * sometimes it's a reset --hard (my changes were made obsolete by a
    different upstream change)
  * sometimes it's a merge
  * sometimes I do nothing.  This is a fairly regular pattern:  I'm in
    the middle of working on something that I know will conflict with
    some changes that were just pushed upstream, and I want to finish
    my changes before starting the rebase.  My collaborator contacts me
    and asks, "Would you take a look at the changes I just pushed?"  If
    I type 'git pull' out of habit to get the commits, then I'll make a
    mess of my work-in-progress work tree.  If I type 'git up' out of
    habit, then the merge --ff-only will fail as expected and I can
    quickly review the commits without messing with my work tree or
    HEAD.

Even if I always rebase or always merge, I want to briefly review what
changed in the remote branch *before* I start the rebase.  This helps me
understand the conflicts I might encounter.

Thus, ff-only always works for me.  I might have to type a second merge
or rebase command, but that's OK -- it gives me an opportunity to think
about what I want first.  Non-ff merges are rare enough that the
interruption isn't annoying at all.

> In a single project, would you
> choose to sometimes rebase and sometimes merge, and if so, what is
> the choice depend on?  "When I am on these selected branches, I want
> to merge, but on other branches I want to rebase?"

My choice depends on the circumstances of the divergence.  It's never as
simple as branch X always has this policy while branch Y has that policy.

> Are there cases where you do not want to either rebase nor merge?
> If so what do you want to do after "git pull" fetches from the other
> side?  Nothing?
> 
> 	Side note: a knee-jerk response to a "yes" answer to the
> 	last question from me has always been "then why are you
> 	running 'git pull' in the first place.

Habit/muscle memory/I'm tired and not thinking 100% clearly.

-Richard

  parent reply	other threads:[~2013-09-05 15:20 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
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 [this message]
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=5228A14B.3000804@bbn.com \
    --to=rhansen@bbn.com \
    --cc=a.krey@gmx.de \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=john@keeping.me.uk \
    --cc=philipoakley@iee.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.