Review Series
On numerous occasions I had to make a case for reviews as I had to introduce them as standard procedure to teams that wanted to improve them. I have decided to put my thoughts on the matter to writing, in the hopes that it might be helpful for others as well.
In Part I of this series, I shall discuss some general guidelines that are useful to keep in mind when trying to foster a healthy reviewer culture. In Part II, I will discuss a more operational breakdown of how reviews might look like in practice.
Part I – The Purpose of a Review
Catch bugs/errors sooner, identify improvement possibilities earlier
The sooner you catch an error in the production cycle, the easier, quicker, and ultimately cheaper it is to fix it. No bug report needs to be filed, no ticket prioritization needs to take place, no investigation into the nature of the bug needs to occur, no need to redeploy.
A quicker feedback loop is desirable in any case, but moving this first feedback to before the code is even considered “done” by the team is even better.
Share knowledge
How many times has it happened that you got stumped with your work, only to later discover that the cause of your problems were some changes in the codebase that you had no idea about?
The review session is a fantastic opportunity for the team to get on the same page regarding the ever ongoing development effort. While dailies may give a general idea about the ongoing features, they are generally not enough to grasp the changes within the codebase. The aim should not just be to accept or reject the changes – by actively participating in a review as much as possible, we can hit two birds with one stone: the changes end up being more familiar, and it fosters a general sense of code ownership, which is imperative for a good product.
How many reviewers should there be
The question then quickly arises – how many reviewers should there be? Clearly, there’s a balance, but generally speaking, we can say that the more active reviewers there are, the better. The review process’ benefits will start to multiply with the number of participants. However, active is the operative word here: the aim is not to have a dozen of developers silently zone out in an hour-long meeting, but rather to have as much back-and-forth between the participants as possible. We will go into more details about the operative parts in Part II.
Formal, recurring timeslot for technical debates
Having an explicit timeslot where developers can discuss technical debates can be very useful. Newly discovered tricks and tips can be shared, new tools can be brought up, and team members can get an opportunity to sync their understanding about the mid-to-long term direction in which the project is going. Therefore, it is encouraged to go above the narrow scope of the code changes at hand, and discuss how they impact the product altogether. It can also function as an early warning system for situations where upcoming requirements might need extra work in the already existing codebase.
(Note that the aim here should not be to come up with new user stories – the review should still aim to answer implementational, technological questions.)
Mentoring
Last but not least, frequent and thorough reviews are a godsend for junior developers. They can receive valuable feedback, ask questions they may have formulated during their efforts, and get a confidence boost that the quality of their work is up to spec.
I feel like it is worth noting 2 things here:
#1: During the review process, we are not trying to judge people, or belittle their work. We are trying to ensure that the work we release will be up to the professional standards that we expect from ourselves. It helps to explicitly separate the finished work from the person doing it – the aim is not to share blame.
#2: Even so, it is very natural to get emotionally attached to changes you have made, and precisely due to this, some reframing can go a long way: it helps if the review is not about “finding mistakes”, but rather, about trying to identify – together – with various ways how the code might be improved. Items from the thus created “Improvement Backlog” can then either be implemented in the next attempt, or be noted as technical debt / bugs to be implemented at a later date.
Final Note – The scope of the review
Above, I have encouraged you to view the review process as more than just the strict acceptance criteria of code changes. However, it is imperative to stress that the scope of the discussions should still be technical and implementational only. The aim is not to come up with new features or user stories – only deal with these inasmuch as they might impact the solution at hand and help us eventually integrate them into our already existing components.
For a more operative approach – along with some examples and details how a review might look like – check out Part II of this Review series.
