Re: Two new Git policy checks

[Owen Taylor <otaylor redhat com>]

On Thu, 2009-04-16 at 18:20 +0200, Santi Béjar wrote:
> 2009/4/16 Owen Taylor <otaylor redhat com>:
> > On Thu, 2009-04-16 at 15:51 +0200, Santi Béjar wrote:
> >> 2009/4/16 Owen Taylor <otaylor redhat com>:
> >> > Check 2: git pull introduced stray merges
> >> > =========================================
> >>
> >> What is wrong with these merges? They are correct and normal if you do
> >> distributed commits.
> >
> > They *may* be correct and normal. But the vast majority of merges of
> > this form I have seen are little pointless:
> >
> >  --A----B-----D--
> >    \       /
> >     ---C--/
> >
> > branches in the history because someone committed C then did a git pull
> > without --rebase.
> 
> But they represent exactly how it was developed. Some people would
> prefer one way or the other, as they have different pros/cons.

In the cases I've seen, B is a .po file commit something - something
entirely unrelated to C. The fact that B and C were done at
approximately the same time is entirely irrelevant to everybody.

I'm not saying that we should force linear history; if someone has a
local feature branch that they merge in, then I'm OK with it showing up
in the history. But what I want to avoid are merges that were unintended
by the committer, and don't reflect development in any significant way.

> > They just make history less readable.
> 
> For sure, a history without merges is more readable. But these kind of
> merge are readable enough.
> 
> >
> > I don't expect many people to be committing to git.gnome.org from a
> > truly fully distributed operation, but if they are, I still wouldn't
> > expect commit messages like:
> >
> >  Merge branch 'master' of ssh://....
> 
> Why not? That is the standard merge commit message with 'git pull'.

I certainly haven't investigated in detail at how git auto-generates
merge messages but certainly in example repositories I've looked at, the
vast majority of such messages seem to be from people running 'git pull'
without arguments to pull from the branch being tracked.

As another example I looked at xf86-video-intel, which is heavily
developed by sophisticated git users, and there were just a few commits
that would have triggered my check, all by two or three committers, and
they were mostly exactly what I was trying to catch, a meaningless merge
over a few commits from pulling from an upstream repo.

[...]

> >> Then they should better check with:
> >>
> >> git push --dry-run
> >>
> >> what would be pushed, maybe with the help of gitk.
> >
> > It's often not a question of not understanding how things work, but
> > rather thinking that you *do* understand it... forgetting that you have
> > some local change. Asking people to exercise great care to not make a
> > mistake we can catch doesn't make sense to me.
> 
> But it is not always a mistake. The only possible mistake would be to
> push unwanted commits and I hope this mistake to be rare.

My take is that the combination of:

 A) People who slipped up
 B) People who didn't know that there was anything they could have
    done instead, and though that funny pointless merge commits were
    just a fact of life.

Is going to greatly outweigh the amount of false-positives. If you are a
sophisticated merger, you can always just change the subject line of your
merge and it will pass fine.

We'll see.

> > and always going to be
> > experienced users who forgot to rebase.
> 
> Maybe they don't want to rebase.
> 
> Let me just present a use case.
> 
> A developer wants to do some "big" changes[1], so s/he decides to
> split them in smaller ones and make one commit with each. S/he has
> tested them in each iteration. At the end s/he is happy with the
> result, so s/he wants to push. But in the meantime other commits are
> pushed to the branch, so s/he has two options:
> 
> 1.- Merge with the remote repository, test the result and push.
> 
> 2.- Rebase on top of the remote repository. But then the tests made to
> the old commits are no longer valid, so s/he has to redo the tests.

I see your point with this example - if you have merge conflicts and
complex interactions with changes done on master, then the rebase might
break bisectability. But I think we would encourage people doing
long-lived sets of interrelated commits to do them on a feature branch. 

- Owen