We have successfully adopted the Gerrit workflow for pre-commit code review in Evojam. We believe that this a foundation of high quality of our solutions. After two years we have worked out a complete approach for the pre-commit review with Gerrit, branching model and application provisioning. Now I have realized the there is no definite cookbook for the newcomers. This led me to the idea that it may be worth to share this approach with the community. This is not totally unique but may save a newcomer a bit of time.

Gerrit-pre-commit-code-review.png

Preface

This guide is extremely detailed. It is aimed at people doing their Gerrit workflow setup right now. If you are already familiar with Gerrit you may jump directly to the workflow description. While writing I took few assumptions about environment you work on:

  • Gerrit is already installed and configured.
  • You have CI, eg.: Jenkins installed and configured, builds are triggered automatically to verify the code
  • You have credentials and privileges in Gerrit
  • The test environment reflects the state of the development branch and is deployed automatically
  • The staging environment is updated manually to Release Candidate versions, tagged on the release branch
  • The Gerrit instance is a central GIT repository
  • The production environment is updated manually from promoted RC builds.

I assume you are familiar with GIT terminology and that you have access to a terminal with git tools ready to use. If you need additional introduction I suggest starting with http://git-scm.com/docs/gittutorial.

In all examples I have used my login abankowski. Please, replace it with yours. For the sake of clarity of the examples I assume that Gerrit is available under https://gerrit.example.com, remember to use your custom domain in urls.

Branching model

Our basic branching model includes only two branches. In rare cases it is acceptable to add feature branches in the central repository, I have described this below in details.

master

The master branch holds the current development code. All changes made by developers, except from hotfixes are prepared upon the HEAD of the master branch. The code in master branch is updated via Change requests. Change requests are merged manually when both conditions are met:

  1. Change receives positive Code Review done by team members.
  2. Code Validation done by Jenkins finishes successfuly.

release

The release branch is maintained to trace production ready code. Code from master branch is merged to release branch once ready to release as a candidate. Usually it can be done with fast-forward strategy.

feature branches

Feature branches are discouraged due to the additional complexity of merging two totally diverged branches. It may be beneficial to create additional feature branch only in special cases. For example when the enormous feature complexity might risk that the development process will take longer than few weeks. In such case a feature branch is going to replace the current master. You may also have a requirement to maintain legacy version of the shared library code and keep the old branch as is.

The feature branch has to be manually created in Gerrit. To do this, user must have appropriate privileges assigned.

The feature branches are subject to all restriction that are applied to ordinary master branch. The development workflow is exactly the same.

Creating feature branch

Open Gerrit in your favorite web browser and navigate to the project management page through Project -> List tabs. Open the Branches tab. Here you can create new feature branch from the master.

Development

Depending if you do a fresh start or have the code already you may want to skip to part A. or B. below.

A. For a fresh start

SSH keys

If you have never interacted with Gerrit you have to add your public ssh key in User Settings. You can find the link to the Settings page when you click on your name in the top right corner of the Gerrit page. If you are not familiar with SSH keys you may be interested in the first two steps from GitHub SSH manual.

Identity

Your identity description in the ~/.gitconfig has to match your account configuration in Gerrit. My [user] section uses my Gmail account, thus I had to update my user settings in Gerrit.

[user]
    name = Artur Bańkowski
    email = artur@evojam.com

Git Aliases

You don't have to add any aliases in your ~/.gitconfig but I recommend you try the following:

[alias]
  push-for-release = push origin HEAD:refs/for/release
  push-for-review = push origin HEAD:refs/for/master
  push-for-draft = push origin HEAD:refs/drafts/master
  unpushed = log --branches --not --remotes --color --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --date=relative
  lg = log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --date=relative
  ca = commit -a
  ci = commit
  st = status
  co = checkout
  br = branch

Some aliases, like ca and ci are just git shorthands I use to make my everyday work more enjoyable.

Cloning

To start working on the code you have to get the copy of the repository to your local computer. Open Gerrit and navigate to the Project page. In the General tab you can find ready-to-use git command. You can either use pure Clone or Clone with commit-msg hook. Second includes the scp call which copies the Gerrit commit-msg hook into your local repository hooks directory. Please, execute the second version. You will get something similar to the example below:

abankowski@laptop:$ git clone ssh://abankowski@gerrit.example.com:29418/demo && scp -p -P 29418 abankowski@gerrit.example.com:hooks/commit-msg demo/.git/hooks/
Cloning into 'demo'...
remote: Counting objects: 2, done
remote: Finding sources: 100% (2/2)
remote: Total 2 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (2/2), done.
Checking connectivity... done.
commit-msg                           100% 4494     4.4KB/s   00:00

What you will get is an ordinary clone of the remote repository in a subdirectory. To start the real work you need a local development branch.

B. When You Already Do Have The Copy

Always pull the current state of the branch from the central repository before you are about to create new local branch. This will allow you to avoid unnecessary code conflicts and rebases. Usually you will be branching from master so for example it will look like this:

abankowski@laptop:~/demo$ git co master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
abankowski@laptop:~/demo$ git pull
Already up-to-date.

Local Branching

Local branches are never directly pushed to the origin so feel free to use the name you like. I suggest the abbreviated ticket subject, eg: filter-aggregation for the task Filters should aggregate data. We have pulled the current branch state from the origin and switched to the master.

abankowski@laptop:~/demo$ git co -b filter-aggregation
Switched to a new branch 'filter-aggregation'

Now with the local branch you can start the real coding part.


The Change

In the Gerrit world the code in the branch on central repository is highly protected. Every time you are about to push some commit into Gerrit you will be preparing a Change. Each commit you push locally to your branch will become a separate Change in Gerrit and will live its own life.

Pushing the Change to Gerrit

Once the code is ready, please verify that it passes all the tests. If you skip this step Jenkins will forbid us to merge the change into target branch.

If you forgot about gerrit commit-msg hook, Gerrit will let you know with the very unequivocal message and a hint how to fix it. You will be asked to execute something like this:

gitdir=$(git rev-parse --git-dir); scp -p -P 29418 \
  abankowski@gerrit.example.com:hooks/commit-msg $/hooks/

Prepare ordinary commit in your local branch. We tend to keep up with the good practice of subject formatting, thus name it by following this convention:

  • Make it concise, keep the length below 100 characters.
  • Use present simple.
  • Remember about the ticket number eg.: 103811230 - Refactor the Search module.
  • If the commit is huge, which is discouraged, add the broader info in the description.

If you have properly copied the Git hook, the commit message will look like this:

123 - I have prepared the test commit

* Here put additional description if applicable

Change-Id: Ie3d0192fd589e845bfbba3bfce13a8a7a2d01198

Do not remove Change-Id unless you have strong confidence how it will affect the Gerrit behavior. Now it's time to push the code to Gerrit. Instead of ordinary git push origin you use an alias push-for-review. The result will be similar to the example:

abankowski@laptop:~/demo$ git push-for-review
Counting objects: 3, done.
Writing objects: 100% (3/3), 306 bytes | 0 bytes/s, done.
Total 3 (delta 0), reused 0 (delta 0)
remote: Processing changes: new: 1, refs: 1, done
remote:
remote: New Changes:
remote:   https://gerrit.example.com/4842 123 - I have prepared the test commit
remote:
To ssh://abankowski@gerrit.example.com:29418/demo
 * [new branch]      HEAD -> refs/for/master

Congratulations, you have published your change with the patchset number 1. Each Change in Gerrit includes at least one Patchset which represents a single successful push-for-review.

Code Validation

Once the commit is pushed to Gerrit as a Change the fully automated code validation process starts. Depending on the build definition, Jenkins runs set of lint checks and tests (please, follow the description for the specific project).

Jenkins will run the tests and rate the change with either +1 Verified or -1 Failed score. If the score is +1 we can proceed with peer Code Review. Otherwise either compilation or tests have failed and you have to fix them. You will find the cause in the build output which is in the log of the Jenkins job. The url is posted automatically by Jenkins in the change history.

Code Review

The code review is done by your teammates. You should never score your own change. In the review process you can either get:

  • -2 Do not submit when the reviewer is certain of some flaws in the change that are not easily fixable and he would like to check the code when you push the fixes
  • -1 I would prefer that you didn't submit this when the reviewers found some minor issues in your change but it can be easily fixed
  • +1 Looks good to me, but someone else must approve when the code is good but he's not confident enough to merge
  • +2 Looks good to me, approved when the code is good enough to merge.

Along with the score you may receive comments for the files included in the change. I encourage you to reply to them, either with simple Done. or with arguments that will make the reviewers change their decision. Remember that comments are in draft until you publish them with Submit button available in the change summary.

When you know you have accidentally pushed the change that should not be reviewed you may score your change with -2 score. It will prevent other developers from reading it, until you fix it and remove the score.

The Success

When the change has no less than +3 score in total from your reviewers and at least one person gave +2 it may be Merged. It does not happen automatically, there is a button for this and traditionally it is done by the last reviewer who gives the positive score. Gerrit will forbid you to merge the commit when the score restriction is not met.

After the merge your commit shows up in the branch the change was pushed to. If it is the master branch Jenkins runs the build and pushes the changed code to the test environment automatically and restarts the applications if necessary. After few minutes you can test the change integrated with the master.

What if it's not good enough

Either you got -1 from Jenkins or from your teammates or the code cannot be fast forwarded automatically by Gerrit you will have to take an action yourself.

Fixing

  1. Update the code, commit message according to the comments you have received. You may try to do this via the Gerrit inline editing but it's often easier and quicker to make it locally.
  2. When you are done, use git commit --amend to update your change. This way, the git history will be clean, the rest will be done by Gerrit. Do not add a separate commit.
  3. Use git push-for-review as you did previously. Your change in Gerrit will get updated with new patchest.

Rebasing

Sometimes, it happens that while you were coding, the target branch (in most cases master) has diverged from the state where you started. To merge the change the rebase is required. Let's assume you have been pushing to master.

  1. Go to the master branch locally and pull from Gerrit, which is usually an origin remote.
  2. Switch your local branch and use git rebase master. You will be able to resolve conflicts.
  3. When you are done use git add and after successful git rebase --continue end up with the commit on top of the current master branch.
  4. Use git push-for-review as you did previously to update the change in Gerrit and publish new patchest.

Releasing an RC to staging

To release an RC you have to merge current master to the release branch.

Merge to release branch

Pull the master and release branches from Gerrit so you can prepare merege. Switch to release branch and use merge command. Don't use fast forward as you need a explicit merge commit to produce new change identifier (the --no-ff switch). This way the branch history is much cleaner for us when the time passes. Remember to change default commit title Merge branch 'master' to release to something related to the release content. See example below.

abankowski@laptop:~/demo$ git co release
Switched to branch 'release'
Your branch is up-to-date with 'origin/release'.
abankowski@laptop:~/demo$ git merge --no-ff master
Merge made by the 'recursive' strategy.
 README.md | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 README.md

abankowski@laptop:~/demo$ git lg
*   7143138 - (HEAD -> release) Release initial version of readme (6 seconds ago) <Artur Bańkowski>
|\
| * c8fe44c - (origin/master, origin/HEAD, master, feature) Unfinished feature (51 minutes ago) <Artur Bańkowski>
| * 136623f - Readme structure (52 minutes ago) <Artur Bańkowski>
| * 0444659 - More details to readme (53 minutes ago) <Artur Bańkowski>
| * 68c0462 - Initial readme (53 minutes ago) <Artur Bańkowski>
|/
* 1843504 - (origin/release) Initial empty repository (65 minutes ago) <Artur Bańkowski>

Unfortunately default githook for ChangeId is buggy, and does not trigger after merge, even with --no-ff switch. To force generation of the ChangeId use git commit --amend without modifying the message.

To publish merge for the review we use the push-for-release alias. The merge commit has to be tested and reviewed as every usual change published to Gerrit. You have to get additional privileges to push merge changes into Gerrit, your sysop should know how to help.

abankowski@laptop:~/demo$ git push-for-release
Counting objects: 1, done.
Writing objects: 100% (1/1), 283 bytes | 0 bytes/s, done.
Total 1 (delta 0), reused 0 (delta 0)
remote: Processing changes: new: 1, refs: 1, done
remote:
remote: New Changes:
remote:   https://gerrit.example.com/4866 Release initial version of readme
remote:
To ssh://abankowski@gerrit.example.com:29418/demo
 * [new branch]      HEAD -> refs/for/release

Push the code to the environment

We use Jenkins to handle the publishing process. Jenkins job tags the current HEAD of release branch with the version you provide and push the RC code to the staging environment. This part highly depends on your provisioning process.

Releasing to production

This task depends on your setup. We use appropriate Jenkins job to publish the code on the production environment. One has to provide the version that is already build from release branch and published as an RC on staging environment.

Hotfixing

When the code in the release branch is already merged by the team it can be released to staging and production as usual. The deployment process is described above. A question arises: how to get the release branch modified without merging current master.

Cherrypicks come for the rescue. Let's look at the common scenarios.

Cherrypick

It often happens that fix can be done on master and then cherry-picked to the release branch automatically. You can do this either directly from Gerrit change view or from your local machine. To do this locally create new local branch from the HEAD of the release and cherrypick the commit into it.

abankowski@laptop:~/demo$ git lg
* dbfd9f5 - (HEAD -> master, origin/master, origin/HEAD, feature) Important hotfix (2 minutes ago) <Artur Bańkowski>
* c8fe44c - Unfinished feature (2 hours ago) <Artur Bańkowski>
* 136623f - Readme structure (2 hours ago) <Artur Bańkowski>
* 0444659 - More details to readme (2 hours ago) <Artur Bańkowski>
* 68c0462 - Initial readme (2 hours ago) <Artur Bańkowski>

abankowski@laptop:~/demo$ git co release
abankowski@laptop:~/demo$ git cherry-pick dbfd9f5
[release 663254e] Important hotfix
 Date: Thu Dec 10 13:46:00 2015 +0100
 1 file changed, 1 insertion(+), 1 deletion(-)
abankowski@laptop:~/demo$ git lg
* 663254e - (HEAD -> release) Important hotfix (2 seconds ago) <Artur Bańkowski>
*   7143138 - (origin/release) Release initial version of readme (54 minutes ago) <Artur Bańkowski>
|\
| * c8fe44c - Unfinished feature (2 hours ago) <Artur Bańkowski>
| * 136623f - Readme structure (2 hours ago) <Artur Bańkowski>
| * 0444659 - More details to readme (2 hours ago) <Artur Bańkowski>
| * 68c0462 - Initial readme (2 hours ago) <Artur Bańkowski>
|/
* 1843504 - Initial empty repository (2 hours ago) <Artur Bańkowski>
abankowski@laptop:~/demo$

When you have cherrypicked your commit locally use push-for-release alias for git, to publish cherry-picked change in Gerrit.

Cherrypick with conflicts...

It may have happen that the bug is nonexistent in the current master due to the massive code refactor or the patch prepared on master is not easily applicable on release branch.

abankowski@laptop:~/demo$ git co master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
abankowski@laptop:~/demo$ git lg
* d6ceccb - (HEAD -> master, origin/master, origin/HEAD, feature) Some change in content (21 hours ago) <Artur Bańkowski>
* ed4483d - Even more content (21 hours ago) <Artur Bańkowski>
* ad0c467 - Continue unfinished feature (21 hours ago) <Artur Bańkowski>
* dbfd9f5 - Important hotfix (21 hours ago) <Artur Bańkowski>
* c8fe44c - Unfinished feature (23 hours ago) <Artur Bańkowski>
* 136623f - Readme structure (23 hours ago) <Artur Bańkowski>
* 0444659 - More details to readme (23 hours ago) <Artur Bańkowski>
* 68c0462 - Initial readme (23 hours ago) <Artur Bańkowski>
* 1843504 - Initial empty repository (23 hours ago) <Artur Bańkowski>

abankowski@laptop:~/demo$ git co release
Switched to branch 'release'
Your branch is up-to-date with 'origin/release'.
abankowski@laptop:~/demo$ git lg
*   914b1fd - (HEAD -> release, origin/release) Second release of bugged version. (34 seconds ago) <Artur Bańkowski>
|\
| * ad0c467 - Continue unfinished feature (21 hours ago) <Artur Bańkowski>
| * dbfd9f5 - Important hotfix (21 hours ago) <Artur Bańkowski>
* |   7143138 - Release initial version of readme (22 hours ago) <Artur Bańkowski>
|\ \
| |/
| * c8fe44c - Unfinished feature (23 hours ago) <Artur Bańkowski>
| * 136623f - Readme structure (23 hours ago) <Artur Bańkowski>
| * 0444659 - More details to readme (23 hours ago) <Artur Bańkowski>
| * 68c0462 - Initial readme (23 hours ago) <Artur Bańkowski>
|/
* 1843504 - Initial empty repository (23 hours ago) <Artur Bańkowski>

We have merged to the release branch after ad0c467 commit and introduced a bugged release.

Prepare the patch on master branch

If the bug exists on the master follow the standard procedure. When the change is merged into master use the cherry pick. Switch to the hotfix-xxx branch based on the current release HEAD and type git cherry-pick {change-id}. The command will fail with conflicts you have to resolve manually. Resolve them, and finish cherrypicking. After you're finished, follow standard procedure of pushing the change to the release branch by push-for-release.

Prepare the patch on release branch

If the bug is not existing in the master branch switch to release and pull. Create your local branch for the hotfix, prepare the commit and use push-for-release to publish. You will have to resolve the conflict during the next release merge.

Common pitfalls

  1. You forgot to pull before creating local branch. Solution is to switch to eg.: master, pull from origin, switch to your local branch and rebase against master
  2. You forgot to branch locally, you have modified the branch, which follows it's counterpart from origin. The rule is: Never modify branches that have set upstream to origin. If it happens, don't panic.
  3. You have prepared multiple commits in your local branch but forgot to push one by one. It may happen that after pushing all at once, the build for the ancestor fails or you get suggestions from your teammates. You can easily fetch a tree from Gerrit in desired state and amend change separately. Furthermore rebasing dependent changes will be required.

Conclusion

We have walked through majority of usual daily tasks that a developer may have to deal with. If you are interested in less technical description of our pre-commit code review, you may read article Overcoming Pre-Commit Code Review Challenges by Michał Nowak. In Michał's post you can read about additional non-technical issues that emerged from our workflow.