A Code Review and Continuous Integration Workflow

As hopefully most people working on software projects nowadays we are writing unit tests and do code reviews.

Work-flow at Splendia

As our project is a large PHP site we are using PHPUnit for unit testing and various static code checking tools (checkstyle, phpmd, pdepend, phpcpd) to verify the quality of our code.

All of these are run on our integration branch, whenever a new feature is integrated.

Before this can happen the code will be reviewed by other people in the team and only if there is a consensus it will be merged into the integration branch.

For the code reviews we are using the pull request system of github. It works very similar to other code review tools, you see a diff view of the changes and are able to add comments to discuss the code.

In these comments we are using a convention of "+1", "-1" and "[B]" to give the pull request a thumbs up, down or mark it as blocked because a critical bug was found. Anyone in the team has a vote and is allowed to discuss any request.

In addition to this the unit tests and some smoke tests will be run on the code of the pull request to avoid merging broken builds.

Only if the tests are successful, there are three positive votes in total and no blocker the request will be merged.

In the beginning all of this was done manually. Developers had to run the tests before the created the pull request and only senior developers had the right to merge a pull request. Every couple of hours they would check the list of pull requests and verify that they had enough votes. It was a distraction and also prone to mistakes.

What gave us a big push in productivity was the introduction of two tools to the work-flow.

ghprb

First the GitHub pull request builder plugin, which enables Jenkins to automatically start a job for each pull request. It has additional features, like starting another build if more commits are added or recognising comments that instruct it to retest or white-list people who are allowed to create builds. It is similar to the pull request feature of Travis-CI only with the full possibilities of Jenkins to your disposal.
With this we are testing every pull request before it gets merged. We also run a limited set of static code analysis, because we want to keep this build fast to give quick feedback. Currently it takes around seven minutes.

PullRequester / plus-pull

The second tool is a simple script which was written by one of our developers and it was called PullRequester. It runs as a cron job and checks whether the pull requests satisfy the +1 and successful test rules. If all is OK it automatically merges it.
I have reimplemented this script to make it possible to publish it. You can find it under the name plus-pull on github. I have added some more features to make it also useful for open source projects.

Final Words

Even though these tools are not magic the increase of our productivity was visible. I account this mostly to way it enabled asynchronous working. No developer has to wait for a senior developer or poke someone to merge his code. As long as he can find three people who agree it will be merged automatically. And as everyone is interested in reviews themselves they tend to happen quickly.

In the screen-shot you can see how this works in practice.