The Talis Approach To Code Reviews
Authors need editors to catch mistakes. It is human nature that one cannot adequately proof-read one’s own work. Authors of software need the same assistance as authors of novels to achieve the goals of the software organization. ~ Smarter Collaborator
As the statement above suggests, humans are not all that good at self-reviewing their work. It doesn’t matter if it’s written as code or plain English, a review by a peer will at the very least raise questions, but more than likely identify errors.
Before I dive into Talis’ approach to code reviews, I should first briefly introduce what a code review is, and why it’s beneficial in the development lifecycle.
What’s A Code Review?
A code review, as the name suggests, is the process of checking (reviewing) code. It usually takes the form of reading through code and assessing if the code is optimal, meets the requirements and is sufficiently tested.
Code reviews are also often referred to as Peer Reviews, and that’s because they are performed by someone other than the author of the code. This is usually colleagues in the same team, that work closely on the same code base, and therefore have a good understanding of the context the code has been written in.
When performing a code review the reviewer should be asking questions of the code along the lines of:
Does this code meet the requirements?
Are the company’s style guides adhered to?
Does it cause any regression failures?
Is there a way this could be improved:
- Could this code be made more efficient?
- Could this code be refactored to make it easier to maintain?
- Is there a design pattern that can be applied?
- Is there sufficient testing?
- Have best practices been followed?
- Does the approach taken make it maintainable?
What’s The Benefit Of A Code Review?
There are many obvious, practical benefits that a code review offers such as helping to ensure the code is fit for purpose, identifying errors before they make it to production and improving the quality of the code. However, in my opinion the primary benefit is the knowledge transfer that takes place.
We work in small teams here at Talis and typically everyone in the team will be assigned as a reviewer to a code review. This means that everyone that’s actively working in the code base is aware of new or modified code. This results in shared understanding before the code is merged into the
master branch. This makes maintainability easier, because hopefully there isn’t a single point of expertise in any given area.
The Talis Approach To Code Reviews
Here at Talis we use GitHub for our code reviews. All code produced here must go through a review before it’s allowed to be merged to the
master branch. To trigger the review process a code author will create a Pull Request on GitHub and assign Reviewers to it.
When do we Code Review?
Typically, code reviews happen once an author has completed the item of work. However, here at Talis we think this is often too late! So we have adopted the approach of being able to trigger code reviews at 30% and 90% complete.
The 30% Review
Early feedback on the outline plan of the changes you are going to make is great! It allows for discussion, clarification, reassurance and often modification before you’ve invested lots of time getting the code into a production ready state. The review is not focused on the details because at this stage of development, the code won’t be polished. It’s purely focused on the approach: does it meet the requirements in the best way?
It also makes suggestions for ‘big change’ to the approach less confrontational. The author has invested less time in their solution and can change direction more easily and quickly. We believe that if you were to wait until the work is complete, the time and cost to make a significant change is too great.
The 90% Review
The 90% code review is perhaps what would be considered more traditional. It occurs at the end of the feature’s development. The code will be more or less production ready, it should be adhering to coding standards and have all tests written. This type of code review is more detailed.
The reason we call it a 90% review is because it’s normally the case that a change is required, and that’s a good thing! It’s the purpose of a review; if no changes were ever made the code review would be redundant.
Something else we do here at Talis is Self Review. This can be done at any point. However, it is often carried out just before requesting reviewers on a Pull Request. The purpose is two fold, first it allows you, the author, to identify errors. You wouldn’t believe how the change of context from your IDE to a highlighted webpage makes mistakes pop out!
Second, it allows us to annotate our code changes, providing a commentary to help guide the reviewer through the decision process the author went through when writing the code. We like to think this makes our code reviews better because reviewers also get some context around the code changes they are looking at.
I concentrated throughout on the critique aspect of a code review. However, a review isn’t limited to ‘…this could be done better like this’_, or _’you have a typo there’. Reviewers often also leave positive feedback. A particular favourite of ours is the virtual ‘Thumbs Up’ ( 👍), to give the author a pat on the back for a job well done.
As mentioned at the beginning, humans aren’t great at self review. To make sure our blogs don’t fall short of our exacting standards even they get the code review treatment. We’re sure they’re the better for it!
Thanks for reading!