Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [egit-dev] Gerrit Commits

Gunnar Wagenknecht <gunnar@xxxxxxxxxxxxxxx> wrote:
> I looked at the new Git URLs via HTTP. I like that way of quickly
> browsing what's going on in a repository. But something that really
> bugged me were a few commits made by a "Code Review" user. I tend to
> call them anonymous commits because there is no indication where it came
> from (just a cryptic id) nor who approved and committed it.
...
> BTW, what are those commits? They don't seem to have a commitdiff but
> they indicate something changed in the repository.

These commits are merge commits.

The project had a temporary fork.  Two different changes were made
on top of the same base commit, and one was submitted into the
branch before the other.  To fix that, Git makes a merge commit
which ties these two branches together.

For example:

  +-- master
  |
  B --------- R
   \
    +--- S

In the above graph, the master branch was at B when both Robin
(R) and myself (S) made two unrelated changes and uploaded them
for review.  Although R was uploaded later in time, it merged in
first, so Gerrit just updated the branch to R:

              +-- master
              |
  B --------- R
   \
    +--- S

When S is submitted, the branch can't be updated to S, because
that would lose R.  So a merge commit M is created to connect the
graph together:

                    +-- master
                    |
  B --------- R --- M
   \               /
    +--- S -------+

gitweb and git show use --cc style of diff formatting when showing
a merge commit (any commit with more than one parent).  In the --cc
style only lines which are not found in any of the parent commits
is shown, these are the lines which were introduced by hand by the
author of the merge and were not inherited from one of the ancestor
branches being merged together.

> It reminds me of some old days where we used to had a build user in
> ClearCase and from time to time people committed changes they made on
> the build machine using that users. IMHO this is really bad style guide
> of using a VCS.

It is.  But in this case, these commits are authored by the Gerrit
software itself.  Gerrit is comparing the delta of B-R and B-S and
making a new tree M out of those deltas.  If a mistake is made,
this is the fault of Gerrit and not of a particular user.

Gerrit actually refuses to accept commits uploaded to it which are
authored by itself, for exactly the reason you state, generic build
users are typically really bad.
 
> Is there any possibility to allow Gerrit to make commits "on behalf of"
> an actual user? I think that "author" and "committer" should be
> meaningful values but not "Code Review <codereview-daemon@xxxxxxxxxxx>".

Well, we could do a few things.

I could change our Gerrit configuration to cherry-pick the change
onto the branch, rather than merge it.  That means that in the
above case we would instead wind up with:

                         +-- master
                         |
   +------------ R' ---- S' 
  /
  B --------- R
   \
    +--- S

Where R' and S' are the same delta as R and S, but have a different
commit SHA-1 than the author created on their system.  It also loses
the fact that S was actually develoepd against B, not against R'.
As such I usually don't prefer this style of submitting changes,
and that is why it isn't the default.


The other thing we could do is change Gerrit's code to try and
attribute those merge commits differently.  Gerrit could look for
the submitter data of the change(s) being merged and try to use that
as the author of the merge commit, instead of the generic build user.

The problem with that style is, some people at my $DAY_JOB disagree
and say these commits are false, they didn't make them, they just
pushed the submit button, the software made the commit on their
behalf.

Although with both an author and a committer line available in the
header we could assign the submitter as the author line, and the
generic user identity as the committer line, to make it clear that
this commit was made by Gerrit on behalf of the user who clicked
the submit button, vs. being a commit the user made on their desktop
and uploaded.

For the latter, this is probably an existing issue in the Gerrit
issue tracker http://code.google.com/p/gerrit/issues/detail?id=162

-- 
Shawn.


Back to the top