Using Gerrit

When using Linaro's Git server, the way we do patch review is using Gerrit.

Gerrit allows us to track and propose changes, do online code review, and merge automatically when there are no conflicts detected.

Linaro's Git and Gerrit servers are basically the same, but with different permission modes on. Some repositories you can commit and push directly (your personal ones), others you can do so on specific branches (your group's ones) and others you can only read from (other groups).

Through Gerrit, you can copy the people who has access to a particular repository and they will have access to "merge" the change automatically via the web interface.

Basic Workflow

Initial Setup

Once using a Git repo from Linaro, you can add Gerrit as your "review" server, by adding a .gitreview file to your repo with the contents:

[gerrit]
host=review.linaro.org
port=29418
project=path/to/repo

That file can be committed to any repo, and once done, just calling a review will setup the remote review repository and send a review to Gerrit.

It is recommended for every repository that will eventually be used with Gerrit to have such file.

Working with Git

Gerrit will not change how git works overall. It will only affect how "git review" works, by sending the reviews to the right server.

However, git review needs a clean tree to work efficiently. Things to be aware:

• Git will create one review per commit, so if you are in a branch with more than one commit ahead of master, you'll end up with multiple reviews.
• Gerrit works by annotating the commit message, adding "Change-Id: HASH". If you update the patch, make sure you keep that line when sending another review.
• Reviews are based on master by default. If you want to push a review to a specific branch, you need to specify "-R branch".
• Gerrit can only see what's committed. Make sure you have added/removed and committed all files in your directory before review.

$ git review

$ git commit --amend

which will update the current commit without creating a new one. You'll have the opportunity to re-write the commit message, adding the changes you've done.

Then, after that, you just need to push it again:

$ git review

and the same Gerrit review will be updated, and people will be notified by email.

Code Review

Once you have sent a commit to review, that commit will have its own review page in Gerrit. That page has a few fields that you'll get used to (see references below), but the important ones are:

• Reply: a button that you press to add comments and reviews, as well as to publish all in-line comments you have done (see below).
• Reviewers: list of people that should look at your code and provide feedback. You have to add them by clicking on the "person" icon on the right. They will be emailed automatically by Gerrit.
• Files: list of all affected files, with links to see the changes in a nice side-by-side comparison. Make sure all the files you need are there.
• History: The list of comments and actions people took on your review, including comments on specific lines.

Once you click on a file, and you get a side-by-side comparison screen, you'll be able to review the changes with red/green colours (indicating added/removed content), where you'll be able to click on a line number and add comments directly to the code.

Those in-line comments do not get published once you save them. You need to go back on the main screen and press the "Reply" button. You'll also be able to post more generic comments about the code as a whole, not specific lines.

In that sub-screen, you'll also see a scale: -2 -1 0 1 2. This is the code review approval/rejection scale, as described below.

You don't need to set those for every comment, only when you're confident that the patch can go in, or has to be blocked, that we add our +1s.

Approval / Rejection

There are 5 types of reviews in Gerrit:

• 0 or neutral: when you have only comments and you're not sure how you feel about a patch.
• +1 or approved: when you're confident that the patch is good, but it would be good to have someone else to look at it.
• +2 or merge: when you're absolutely confident that no one else needs to see the patch to merge.
• -1 or cautious: when you don't think the patch is good, but it can either be made better, or you could just be wrong about it.
• -2 or refusal: when you're pretty sure the patch is bad or completely broken and no one else should approve before convincing you.

However, these statuses have different meanings, depending on which repository / branch the patch is proposed to.

For example, patches to personal repositories/branches will not break any production system/development and can usually be approved directly, even if refused by someone else on the team. A strong refusal (-2) in this case means the reviewer is very uneasy about this change going into master / production, not necessarily staying on your personal branch.

In contrast, a cautious review is a real blocker if targeted at a branch that is either publicly available officially, or serving a production environment (like Jenkins), and reviewers who are cautious need to be convinced and need to change their statuses to either neutral or approved.

Gerrit has wide customisation to allow groups of people able to approve or reject, but given the wide interpretation of what they mean, any stronger enforcement would have a more negative effect than provide safety. For this reason, we allow any developer in the team to have full rights, and we expect people to know how to use and to be considerate.

Given how easy it is to manipulate Git repositories, accidents can be fixed mostly painlessly, so allowing for a bit extra power for our engineers pays off in the long term.

How many +1s equal one +2?

Some teams have rules like: "2 +1s are equal 1 +2". Meaning that, if two people +1, another can +2 and merge.

Some teams consider only reviewers' +1s, others allow for the author to +1 too (meaning any +1 means merge).

Mostly, these rules depend on what kind of repository we have and what we use it for. Ideally, we should have different rules for different use cases, but this complicates things.

So, to simplify, we should follow the simple rule that, if someone else added a +1, you can +2 and merge.

If that rule proves problematic, we can review on a case by case basis.

Self Approval

As mentioned above, it's ok to approve your own patches on a repository that you are developing by yourself, or on a local branch. Given that Linaro's Git doesn't allow certain types of pushes (mostly to master branches on official repositories - ex. leg/hpc/*), Gerrit is our only choice.

Self approval in Gerrit can also be used to let people know of a patch you wrote, by pushing for review, copying them, approving +2 and then merging them directly.

It should not be used, however, on repositories that are being developed by multiple people, or are in production, etc. Common sense apply here.

References

Gerrit's own webpage

Basic usage of Gerrit

Advanced usage