Peer Review
Chris Didcote
At ComputerMinds we like to think that we’re all pretty good at what we do; however, nobody is perfect and this is why we always ensure that our code is properly peer reviewed as part of our quality assurance process.
Peer review is literally just what the name implies; we work together to review each other’s code to make sure that it all makes sense. This approach means that we’re able to spot obvious mistakes before they become a problem. It also has the huge advantage of allowing us to transfer knowledge between our team on a day-to-day basis.
###Pull Requests
The primary way we peer our code is to make use of GitHub’s pull requests (PR) feature. This means that whenever we need to do some work on a Git repo we start by creating a new feature branch which will contain the chunk of work that we’re doing. Then once we are happy with the code we’ve written in this branch we’ll go over to GitHub and create a PR to merge our branch in with another branch which we know is stable, for example the master branch. Before this merge happens GitHub’s PR tool will show all the changes between the the two branches so that they can be reviewed by another developer.
At ComputerMinds we use pull requests a lot. We don’t like to work directly on a stable branch as this way there is much more chance the bugs might slip through the net. By using pull requests we can be sure that our code is properly sanity checked before it makes its way over to a stable environment, be that a client facing testing branch or the live branch. GitHub also makes it easy to add comments directly to the pull request so any issues are full documented and feedback is clearly displayed.
###Face to face
When dealing with a more in-depth code change, it's particularly helpful to talk face-to-face, as it allows the original developer to talk you through their changes and the thinking behind them. This allows the reviewer to have a much better understanding of what the original developer was aiming to achieve and to sanity-check their thinking. A 'meatspace' chat can be more difficult to achieve than just getting some comments on a pull request, but it's often worth the effort.
###Finding the right fit
Both of these methods have their strengths and weaknesses. Pull requests are quick and easy to use; however, when dealing with larger sets of changes things may get overlooked, or may not be properly understood without knowledge of the bigger picture. Face to face reviews obviously take up more resources to conduct the review but do allow for a more in-depth review where the bigger picture can be clearly explained by the original developer.
Obviously it goes without saying that these two approaches to peer review aren’t mutually exclusive - there are plenty of meatspace chats going on around the office about various PRs.
At ComputerMinds we're still working on how we do code review. There's always room for growth and for change, and we're actively promoting discussion amongst our team to see how we can do better.
How do you do quality assurance and review on your code? Share your thoughts and tips with us below!