Re: Two new Git policy checks

[Santi Béjar <santi agolina net>]

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.

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

>
> The check actually looks for the URL form like that. Because I'd expect
> you to add remotes for your collaborators and fetch and merge those
> remotes. As a data point, git.git doesn't have a single commit that
> would trigger this policy test.

The merges with gitk are of the form:

Merge git://git.kernel.org/pub/scm/gitk/gitk

it is not "Merge branch 'master' ..."  because it merges the default
branch (HEAD).


http://git.kernel.org/?p=git/git.git;a=commit;h=f8c62880ef22b74ea6df47bb349ff0743d2a93f9

>
> If it does become a problem we can make the check more specific and look
> for (ssh|git)://git.gnome.org instead of arbitrary ssh URLs.
>

OK. Then we'll see the details of the problem.


>> > If you look through the commit lists, we are getting a lot of these
>> > stray merges, even from people that should know better :-)
>>
>> They are not bad per se. Are they *all* introducing extra unwanted
>> commits?
>
> As far as I saw every merge of this form was extraneous.

For a certain definition of extraneous.

> There were no
> merges of repositories other than git.gnome.org, there were no merges of
> branches other than the branch the committer was working on. And there
> weren't even any conflicts.

For me these are not arguments for forbidding these merges.

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

>
>> Mistakes happen, but they should no harm the workflow for everybody.
>
> If people's workflow is harmed, I'm sure they will complain and we can
> figure out better and more specific checks. But I want to get something
> in place that will prevent accumulating a lot of these merges.

I don't see the point of avoiding these merges, but I'll wait to see
it the hook is really a problem in practice or not.

>
>> Maybe it is OK to do it while the people learn the new system, but no
>> definitely.
>
> There are always going to be new users

But the percentage of new "users" is higher now.

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

And this is working with git.gnome.org only.

Santi

[1] But remember that it is better "release early, release often" than
a big code drop. And this something that is not checked (I don't know
exactly how it can be checked) neither documented. And with
distributed VCS it is easier to maintain a local branch, but it goes
against the GNOME way and should be avoided somehow.