[Insight-developers] My experience with gerrit

[Brad King]

On 08/25/2010 12:29 PM, Bradley Lowekamp wrote:
> remote: (W) 26c0e7: commit subject >65 characters; use shorter first
> paragraph
> remote: (W) 26c0e7: commit message lines >70 characters; manually wrap lines
[snip]
> First I am not sure what the issue is with the commit message. I write a
> short first line and then a long description ( no wrapping ), like I
> have been told is good practice with git.

Gerrit is apparently more strict about commit message formatting than
our hooks are (but only warns).  It wants the first line to be at most
64 characters (our hooks enforce <= 78).  It also wants all the lines
in the free-form section to be <= 70 characters (our hooks enforce
nothing here).

I think it is safe to ignore these warnings.  However, I do suggest that
you format the free-form part of commit messages as paragraphs unless
there is a need to do pre-formatted ascii-art or something.

> remote: New Changes:
> remote:   http://review.source.kitware.com/3
> remote:   http://review.source.kitware.com/4
> remote:   http://review.source.kitware.com/5
> remote:   http://review.source.kitware.com/6
> remote:   http://review.source.kitware.com/7
[snip]
> Each commit came up as a separate issue. I did not expect this. Perhaps
> my commit are too small, but this was not the result I expected.
> I am not sure the branch thing really works with gerrit. This topic is
> to be merged into master ( I will normally use the staging area to
> perform this ). I suppose gerrit is for the commit level review  and not
> the branch?

Marcus and I have discussed this at length.  Gerrit review is commit-based
rather than topic-based.  We've discussed ideas on how to adapt Gerrit to
provide topic-level review but have not yet generated a concrete proposal.

I agree with your micro-commit preference.  It is much nicer, and I would
hate to abandon it just for Gerrit.  Gerrit does at least keep track of
the dependencies between commits.  Until topic-level review is available
we'll have to use it as-is.

-Brad K