Last week, we kicked off the Apps Factory’s DevOps blog series with a discussion of version control systems (VCS) for startups.
In this post, we’ll look at how the peer review system can benefit rapid-development and startup teams, and provide an analysis of various solutions that might help you.
While we focus on peer review systems that support Git development, there rarely is a one-size-fits-all solution to code quality, so we’ll also look at “pair programming” as a possible alternative.
Why Peer Review?
The goal of peer review is to improve code quality, which can increase your product’s value proposition. While there are standards (ie. IEEE Std 1028) and certifications that large enterprises aspire to achieve when it comes to code quality, startups lack such pressures. With time being an obvious challenge, startups face a strong urge to push a product to investors and potential clients as quickly as possible. In my experience working with startups, developers want code review, but their managers think it’s not worth the expense.
The concept of technical debt is a counter-argument to that kind of thinking. Technical debt describes the amount of work that needs to be done before a job can be considered complete. As the product moves through the lifecycle and incurs “interest”, it increases in complexity, and if not properly maintained, the technical debt rises. Here is an interesting illustration of the debt analogy:
“Shipping first-time code is like going into debt. A little debt speeds development so long as it is paid back promptly with a rewrite. The danger occurs when the debt is not repaid. Every minute spent on not-quite-right code counts as interest on that debt. Entire engineering organizations can be brought to a standstill under the debt load of an unconsolidated implementation.” – Ward Cunningham, WyCash Portfolio Management System
We’ll look at the various forms of peer review in the next two sections. While pair programming focuses on the way the code is written, there is opportunity to integrate code review into this method, but some basic pre-conditions must be met:
- At least one developer who adheres to principles of good architecture design, or at the very least, avoids these bad design principles!
- One coffee break worth of time a week
Peer Programming - Is it enough?
The concept of pair programming implies that two developers are working on the same machine. To use a driving analogy, the driver codes while the navigator reads, checks and sanity-tests the code while deciding how to tackle the next problem.
Advantages include:
- Programmers can learn skills from each other
- Better awareness of code architecture reduces maintenance downstream and allows the team to focus on developing product instead of fixing bugs
- The bigger the team, the greater the opportunity to share ideas, which results in more innovative products
- Writing code beside someone is a perfect motivator to work harder and write better code.
Things to consider:
- Do you have enough resources to allow for pair programming?
- What is the personality mix of your team? Can your developers work effectively with others?
- What is the technical mix of your team? Are your developers individual specialists, or are they broad enough in their knowledge to enjoy economies of expertise by working with other developers?
- How stable is your team’s structure? Is the retention rate
lowhigh enough you can utilize the knowledge sharing that comes from peer programming?
Git Best Practices - Foundations for Peer Review Tooling
At the Apps Factory, we follow these best practices to better manage our peer review process:
Use feature branches
- One of Git’s advantages is cheap, fast, local branches. If you’re working on a new feature or fixing a bug, you can separate that work into its own branch until you’re ready to merge it back into your developer branch.
Conduct post-commit reviews
- The flexibility of feature branches allows developers to work on other code while changes are being reviewed. Post-commit reviews provide metadata in Git to allow for tracking and to revert work when changes are needed.
Commit rework in your feature branch
- If there are defects in your review, you’ll need to rework your code to fix the defects before the review can finish. Perform these reworks in the feature branch you used to create the original review. This ensures that your changes are always based on the code that is under review.
Prefer mergers over rebasing
- When your review has been accepted, it’s time to include your changes in your master branch. This can be done in two ways: Rebase or Merge. When you rebase, Git creates new commits that have the same code changes and metadata, except the commit IDs are different. This means that, when you’ve committed a change for review, the rebased commit will not match, making it potentially difficult to tie code reviews to Git commit.
Disclaimer: While some of these points are often the topic of heated conversation, some of these practices don’t apply to all startup teams, as it largely depends on context!
Code Review Tools - Available Open Source Tools
ReviewBoard (http://www.reviewboard.org/)
Supporting both concepts of pre/post-commit, ReviewBoard works well when a team has a central “official” git repository (ie. GitHub). ReviewBoard requires a team to download and set a local instance of the solution. It supports the following basic workflow:
- Clone the central repo
- Create a local branch and make a change you want reviewed
- Run post-review (a ReviewBoard-specific tool you install), comparing the branch to master
- Get reviews, make additional changes on the branch as needed
- When the change is marked to ship, merge it into master and push it to origin
ReviewBoard is different in that it supports the concept of pre-committing changes to be reviewed before they are committed to a branch. A more strict way of enforcing code quality, it can be used to strengthen code stability in a repo. Pre-commit git hooks support this workflow:
- Clone the central repo
- Make a change you want reviewed, but do not commit it yet
- Run post-review
- Get reviews, update your change as needed
- When the change is marked to ship, commit it to master and push it to origin
While the flexibility of ReviewBoard allows it to work with any workflow, it does present two constraints:
- It doesn’t support any plug-ins for continuous integration tools such as Jenkins and Hudson which help to verify code quality (see verification step for Gerrit).
- Complex customization is required to support forked repos as suggested by GitHub.
GitHub (https://github.com/)
A web-based hosting solution for Git-based projects, GitHub offers features found in most code-review tools:
- Easy-to-use dashboard
- Detailed review request
- Powerful diff viewer with inline commenting
- Contextual discussions and reviews which can be attached to commits
It promotes the forking mechanism (a local copy in a user’s own github account) as both a lightweight workflow and a way for teams to stage their code. In other words, to review and assemble code before it’s ready to be in production. One example can be described using this model:
*github/dev1 and github/dev2 are forked repos of github/AppsFactory. A forked repo usually is not expected to be merged back upstream
Within this context, GitHub also advocates the use of the “Pull Request” feature which can be seen as a light implementation of the final step of code review while providing some administrative control. Cost is a drawback to GitHub. The free service offering only applies to publicly accessed repositories, and private repos come at a cost. Still, its offerings are quite comprehensive and a worthwhile investment for any team.
Gerrit (https://code.google.com/p/gerrit/)
Originally designed for the Android project, Gerrit is a neatly packaged solution that offers much more than just a code-review dashboard. Similar to GitHub where it offers an intermediary staging step, a team needs its own internal server (like ReviewBoard) to run this tool. Here is a sample workflow:
Similar to GitHub, Gerrit can use various plug-ins that support continuous integration solutions such as Jenkins (http://jenkins-ci.org/) for improved testing performance. This two-step process occurs in the “Pending” state within Gerrit:
- Review: By design, this step requires a gatekeeper, such as a product owner/manager, to look at the code and ensure it meets project guidelines before signing off on the patch.
- Verification: This step ensures tests have passed and that the code actually compiles. This step can be done manually by the gatekeeper or automatically through continuous integration tools such as Jenkins or Hudson.
As with all tools presented in this article, there isn’t a “best tool”. Trade-offs need to be considered and often times just trying out a tool to see how it works with your team is the best way to determine which is the most appropriate.
Fit with next topic
How can you take peer review to the next level? Our next post will discuss continuous integration and the automated tools that can make the verification and release steps of your development process painless and quick!