What is it?
Code review is a process we use as part of our development to increase the quality of the code we write and the products we create. Quite simply, it’s just a developer looking at someone else’s code and making any relevant comments. It’s hard to explain what it is without also saying why we do it and how we do it, so read on...
Why should I do it?
There are lots of reasons for reviewing code. The most obvious is to find bugs as early as possible and it can also be used to enforce consistency in code style and architecture. Where possible, it’s often better to use automated tools to check code style, such as checkstyle for Android and SwiftLint for iOS.
Probably the main reason we use code review is that it’s a good tool for learning. It’s an effective way to teach each other, particularly senior developers reviewing code by a junior developer, but also the person doing the review can learn from the code that they’re reviewing.
How should I do it?
We use Reviewable to do our code reviews. You can work through the changes one file at a time, marking a file as reviewed or adding questions and comments as necessary. When you’re ready you can publish your review and the comments will be sent to the author for review. The author can reply to a comment or mark it as resolved, and you can review any new commits added to the branch in isolation from all the changes that you have already reviewed.
This process is started by creating a pull request on GitHub.
Who does it?
We say that all code should be reviewed regardless of the author. This is partly because anyone can make mistakes but also because we can all learn from each other - we want people to learn from being reviewed and from reviewing others.
When should I do it?
Code should be reviewed when it is considered finished, otherwise you’ll be reviewing code that will probably change or may be removed. For us this is normally when a Jira task is done, but this can lead to reviews that are bigger than is ideal. In this case the task should be split into subtasks so that the subtasks can be reviewed separately. We try to limit code reviews to around 400 lines.
What to look for
Each company will have its own priorities for what to cover in a code review, whether it’s keeping code consistent by doing a certain thing the same way, or a preference for or against a certain language construct e.g. do you love Swift’s guard statement, or ban unnamed tuples? Use your experience of issues you’ve had to solve in the past or things you’ve learnt that make coding easier to suggest improvements. Over time you’ll be able to refine what works best for you. If you team finds common issues, see if you can automate that part of the review.
One thing worth considering is code reviewing yourself. Whenever you’re about to do a commit, look at the code you’re committing. Review the code to see if you’ve missed anything. Make sure you haven’t left in any changes that shouldn’t be committed. It’s also a good time to look for anything that could be improved, such as refactoring a long method.
Personal annoyances are unnecessary whitespace changes which just bloat the code diff, and any Xcode xib files that you haven’t actually changed. Don’t commit those annoying version changes when a new version of Xcode is released or all those “misplaced” views that Xcode is temperamental about.