Code review proposal: gerrit

Development
1

Hi all,

Marcus and I have been discussing code review options on IRC and
private email, and the gerrit code review system looks promising. It’s
made for git, and should be good for what we need:
http://code.google.com/p/gerrit/

Here’s a video that has a bit of background on the project:

And a rather detailed overview (with screenshots):
http://unethicalblogger.com/posts/2009/12/code_review_gerrit_mostly_visual_guide

To summarize how it works, I’ll paraphrase from the third link:

Developers set up gerrit as a remote git repository, and to submit
changes for review, just push the to remote, e.g.

#set up the gerrit repo
git remote add gerrit ssh://gerrithost/project.git

Create a topic branch

git checkout -b topic-branch

do work and make commits

Push to gerrit server

git push gerrit HEAD:refs/for/master

Now the gerrit web interface will show that there is a new patch ready
for review, and can optionally send emails to the project
“integrators” who can review and approve the code. There are
side-by-side and unified diffs available on the web interface.

If there is a problem with the commit, the reviewer can discuss it
within gerrit or via email, and an iterative process occurs until the
commit is approvable.

Once the changes are approved, the integrator can push the changes to
gerrit/master to show them as accepted:

banana% git merge topic-branch
banana% git push origin master
banana% git push gerrit master

This automatically closes the submission on the web interface.

It has some features similar to what we’re used to on github:
“Gerrit contains a number of nice subtle features, like
double-clicking a line inside the side-by-side diff to add a comment
to that line specifically, the ability to “star” changes (similar to
bookmarking) and a too many others to go into detail in this post.”

From the screenshots and description, I think this would clear up a
lot of the confusion and problems with our current review process. It
also looks like gerrit does a lot of work behind the scenes from this
flowchart:
http://source.android.com/source/life-of-a-patch.html

Thoughts?

2

On Sat, May 29, 2010 at 10:01 PM, David Lonie loniedavid@gmail.com wrote:

Hi all,

Marcus and I have been discussing code review options on IRC and
private email, and the gerrit code review system looks promising. It’s
made for git, and should be good for what we need:
Gerrit

Here’s a video that has a bit of background on the project:
http://www.youtube.com/watch?v=VzkudNGUepQ

And a rather detailed overview (with screenshots):
unethicalblogger.com

To summarize how it works, I’ll paraphrase from the third link:

Developers set up gerrit as a remote git repository, and to submit
changes for review, just push the to remote, e.g.

#set up the gerrit repo
git remote add gerrit ssh://gerrithost/project.git

Create a topic branch

git checkout -b topic-branch

do work and make commits

Push to gerrit server

git push gerrit HEAD:refs/for/master

Now the gerrit web interface will show that there is a new patch ready
for review, and can optionally send emails to the project
“integrators” who can review and approve the code. There are
side-by-side and unified diffs available on the web interface.

If there is a problem with the commit, the reviewer can discuss it
within gerrit or via email, and an iterative process occurs until the
commit is approvable.

Once the changes are approved, the integrator can push the changes to
gerrit/master to show them as accepted:

banana% git merge topic-branch
banana% git push origin master
banana% git push gerrit master

This automatically closes the submission on the web interface.

It has some features similar to what we’re used to on github:
“Gerrit contains a number of nice subtle features, like
double-clicking a line inside the side-by-side diff to add a comment
to that line specifically, the ability to “star” changes (similar to
bookmarking) and a too many others to go into detail in this post.”

From the screenshots and description, I think this would clear up a
lot of the confusion and problems with our current review process. It
also looks like gerrit does a lot of work behind the scenes from this
flowchart:
http://source.android.com/source/life-of-a-patch.html

Thoughts?

We have to try it but it sounds amazing :slight_smile: One question though, do
you create one gerrit repo or are there multiple ones. If a single one
is needed, did you create one already? Can’t wait to try it…

Tim

Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
avogadro-devel List Signup and Options

3

On Sat, May 29, 2010 at 4:59 PM, Tim Vandermeersch
tim.vandermeersch@gmail.com wrote:

We have to try it but it sounds amazing :slight_smile: One question though, do
you create one gerrit repo or are there multiple ones. If a single one
is needed, did you create one already? Can’t wait to try it…

I haven’t tried it yet, it just sounded interesting and wanted to know
what everyone thought. I’m thinking about setting one up for another
project tomorrow, and I can set up a trial avogadro one so everyone
can play around with it and get a hands on feel for it.

Also, if anyone can recommend a different review system, please do!
I’m by no means fixed on gerrit, I just like the git integration :wink:

Dave

4

On Sat, May 29, 2010 at 5:04 PM, David Lonie loniedavid@gmail.com wrote:

On Sat, May 29, 2010 at 4:59 PM, Tim Vandermeersch
tim.vandermeersch@gmail.com wrote:

We have to try it but it sounds amazing :slight_smile: One question though, do
you create one gerrit repo or are there multiple ones. If a single one
is needed, did you create one already? Can’t wait to try it…

I haven’t tried it yet, it just sounded interesting and wanted to know
what everyone thought. I’m thinking about setting one up for another
project tomorrow, and I can set up a trial avogadro one so everyone
can play around with it and get a hands on feel for it.

Also, if anyone can recommend a different review system, please do!
I’m by no means fixed on gerrit, I just like the git integration :wink:

I have a sandbox on my home server set up:
http://drunks.servebeer.com:8080 I’ve encountered a few errors, but
from the logs it looks like I have a buggy mysql setup, and gerrit
itself is not causing them.

I made some trial change sets to play around with. Click “All”->“Open”
will show the commits that need review. Clicking on them will show the
commit’s history, any comments, and side-by-side diffs of the files
affected. Clicking review (if the user has proper permissions) allow a
reviewer to either vote +1 in favor of accepting the change or -1 in
favor of not accepting the change.

Once a change gets a threshold score (default is +2 I think), it moves
into verification, where someone with write access to master attempts
an integration (I’ve read that gerrit may do this automatically, but
I’ve not tried). If the changes apply cleanly, it can be merged into
master.

Some other review systems have come up on IRC (feel free to contribute more):

  • Reviewboard
    pros: used by kde, nice bugzilla integration (many of us aren’t fans of SF…)
    cons: patches must be created by hand and uploaded

  • Launchpad
    pros: total package – bugs, review, etc
    cons: Kinda heavy for what we need, and code management would require
    syncing with bzr

*Gerrit
pros: lightweight, easy to submit changes for review (‘git push gerrit
HEAD:refs/for/master’. Done.)
cons: sparse documentation (a detailed wiki page would be necessary)

Dave

5

On Saturday 29 May 2010 16:01:24 David Lonie wrote:

Hi all,

Marcus and I have been discussing code review options on IRC and
private email, and the gerrit code review system looks promising. It’s
made for git, and should be good for what we need:
Gerrit

> > lot of the confusion and problems with our current review process. It > also looks like gerrit does a lot of work behind the scenes from this > flowchart: > http://source.android.com/source/life-of-a-patch.html > > Thoughts? > I have been watching the video, and looking through some of the links. It certainly looks like a great tool to me. I had some doubts about whether it was possibly overkill, but I would love to try this out and see how well it works.

Even from a few early email conversations I am convinced that this will
improve Avogadro. I have been wanting to use code review in Avogadro for
years. I honestly think it is worth slowing development progress a little (not
stalling) for the improvements in quality we will realize.

It does depend on participation. So I think keys are submitting topic branches
when they are ready, discussing them, may be reworking, and then integrating
them. Combine this with some more dashboards for at least cross platform build
testing, and we could be onto a real winner.

Thanks,

Marcus

6

On Sunday 30 May 2010 08:50:37 David Lonie wrote:

On Sat, May 29, 2010 at 5:04 PM, David Lonie loniedavid@gmail.com wrote:

On Sat, May 29, 2010 at 4:59 PM, Tim Vandermeersch

tim.vandermeersch@gmail.com wrote:

We have to try it but it sounds amazing :slight_smile: One question though, do
you create one gerrit repo or are there multiple ones. If a single one
is needed, did you create one already? Can’t wait to try it…

I haven’t tried it yet, it just sounded interesting and wanted to know
what everyone thought. I’m thinking about setting one up for another
project tomorrow, and I can set up a trial avogadro one so everyone
can play around with it and get a hands on feel for it.

Also, if anyone can recommend a different review system, please do!
I’m by no means fixed on gerrit, I just like the git integration :wink:

I have a sandbox on my home server set up:
http://drunks.servebeer.com:8080 I’ve encountered a few errors, but
from the logs it looks like I have a buggy mysql setup, and gerrit
itself is not causing them.

http://gold.cryos.net:8080/

After talking with David, and hitting a few problems on his machine, I set up
a new sandbox. This can be the new play area to get a feel for it, if it works
out I am happy to host it, or we can look into other hosting options.

Looks like migration from one server to another shouldn’t be too difficult,
although with the heavier Java processes many of the shared hosting options
may not be happy to have it. I can keep an eye on load, but that is a quad
core server and so should hopefully be able to deal with it.

One thing you need to do is add your committer email address, it picks out
your GMail address by default. After that, it was pretty easy to add a
username/SSH key and upload changes. I am just feeling my way through the
review process/merging and eventual syncing with other repos.

Please play, let us know what you think, any problems you hit with it.

Marcus

7

On Sun, May 30, 2010 at 7:43 PM, Marcus D. Hanwell marcus@cryos.org wrote:

On Sunday 30 May 2010 08:50:37 David Lonie wrote:

On Sat, May 29, 2010 at 5:04 PM, David Lonie loniedavid@gmail.com wrote:

On Sat, May 29, 2010 at 4:59 PM, Tim Vandermeersch

tim.vandermeersch@gmail.com wrote:

We have to try it but it sounds amazing :slight_smile: One question though, do
you create one gerrit repo or are there multiple ones. If a single one
is needed, did you create one already? Can’t wait to try it…

I haven’t tried it yet, it just sounded interesting and wanted to know
what everyone thought. I’m thinking about setting one up for another
project tomorrow, and I can set up a trial avogadro one so everyone
can play around with it and get a hands on feel for it.

Also, if anyone can recommend a different review system, please do!
I’m by no means fixed on gerrit, I just like the git integration :wink:

I have a sandbox on my home server set up:
http://drunks.servebeer.com:8080 I’ve encountered a few errors, but
from the logs it looks like I have a buggy mysql setup, and gerrit
itself is not causing them.

http://gold.cryos.net:8080/

After talking with David, and hitting a few problems on his machine, I set up
a new sandbox. This can be the new play area to get a feel for it, if it works
out I am happy to host it, or we can look into other hosting options.

Looks like migration from one server to another shouldn’t be too difficult,
although with the heavier Java processes many of the shared hosting options
may not be happy to have it. I can keep an eye on load, but that is a quad
core server and so should hopefully be able to deal with it.

One thing you need to do is add your committer email address, it picks out
your GMail address by default. After that, it was pretty easy to add a
username/SSH key and upload changes. I am just feeling my way through the
review process/merging and eventual syncing with other repos.

Please play, let us know what you think, any problems you hit with it.

Just to chime in here, a good tutorial of how to use gerrit is
http://wiki.scilab.org/gerrit

Dave