I was browsing Twitter a while ago and came across a great tweet about trying to include nice, positive things within code reviews. It got me thinking about how we, as an industry, tend to review the code of our team or peers and how we can improve that process.
Can we start a trend?
In every code review, make sure to say at least one nice thing about the person's work. People tend to provide almost exclusively criticism, because praise feels awkward to deliver. But it becomes easy with practice!
— Robert Arkwright (@arkwrite) June 2, 2018
What’s the problem with code reviews?
I’ve been in development for close to 20 years now and worked in a variety of places in a variety of roles, transitioning from full-stack developer to the front-end coder I am today. In that time, I’ve seen all manner of code review processes ranging from nothing at all-to cursory eyeballing, right across to full-blown microscopic nick-pickery.
The main reasons I’ve discovered that we tend to avoid code reviews or do them badly as developers are very simple:
- Perceived lack of time to fit them in
- Perceived lack of value in the process
- Lack of code review guidelines
- The human condition of skewing towards the negative
I’m going to get to these in turn, but first, let’s take a look at why code reviews (good ones) are a vital part of the development process and contribute to stronger teams, better software and all-around awesome development habits.
Why code reviews are necessary
One of the biggest issues with development is that we tend to tackle our work in isolation. Even within a team, unless we’re stuck and ask for help or there are some mandated meetings (e.g. Agile standups), we tend to take our tasks away to our desk and solve for x. Because each person is different, each approach and solution produced can be quite different from other developers in the team.
Diversity is great, but by straying too far from agreed development guidelines, we can introduce more technical debt and maintenance headaches in the future. Also, when working in a typical developer vacuum, we’re so focussed on producing and delivering a feature, that we can lose some objectivity about how things should function and interact. This leads to errors and bugs. We’ve all been there!
Code reviews offer a dedicated period of time for at least one other pair of eyes to work through your code and (at the very least) sense check it. Good code reviews should accomplish some of the following goals:
- Give the code producer time to step back and reflect on what they’ve developed
- Give team members a chance to see how other team members are progressing and how they’ve tackled a task
- Catch some coding errors before merging or before the QA team give things a realistic run-through
- Ensure style guides and coding conventions are adhered to
- Highlight successes and interesting approaches to problems
- Discuss opportunities for improvements or refactoring
- Objectively test at the development level with a fresh set of eyes
- Break the development vacuum
- Facilitate good team communication habits
What bad code review habits look like
We should take the time to look at the main causes of poor code review habits and try to understand where some of the attitudes come from and how we can change them.
Lack of time
Avoiding reviews altogether or giving cursory, rushed reviews can sometimes come down to a lack of time to dedicate to the process. We all get busy, we all neglect things, deadlines hit us all hard from time to time. However, skipping out on such a valuable part of the development cycle means that we lose so many of the benefits we already discussed.
Sometimes, it can be a mere perception that there isn’t enough time to fit everything in and that code reviews can take too long. However, a good review can take as little ten minutes and this is a fraction of time to sacrifice to reap some worthy rewards.
No value in the code review process
Usually, this attitude is found at higher, management levels, or amongst some of the more grumpy, dyed-in-the-wool developers who dislike change. There is definitely a huge amount of value to be had by scheduling regular code reviews between members of the team. That said, it’s easy to see why this perception exists; after all, bugs still slip through the net, and even the best reviews don’t physically move the development needle — they don’t add features or fix errors.
But there are lots of tools in the development shed that doesn’t directly contribute to building a feature or adding a widget, but they do positively contribute in part to a larger whole.
Lack of review guidelines
Like any good process, documentation and guidelines are key. Without a set of starting points, people are left to their own devices; some will shine, but others will struggle and experiences will ultimately vary between person to person. By bringing the development team together, they can collectively agree upon a good set of habits and guidelines for processes like code reviews.
It doesn’t have to be about endless rules that mustn’t be broken either! Even a simple set of guidelines can help give the team a frame of reference, which could include:
- How long a review should last
- How they should be carried out (e.g. in person, via GitHub or similar)
- What the outcomes should be
- How many people should be involved
Skewing towards the negative
Whichever way you spin it, part of the review process will sooner or later involve criticism. The difficulty is, as humans, we tend to skew towards the negative and it’s easy to avoid criticism on the receiving side to reduce confrontation, whilst becoming a little subjective on the giving side. Perhaps it’s that some team members don’t see eye to eye and things can get a little personal. Maybe it’s a case of not agreeing with someone’s coding approach and becoming negatively biased against it.
Challenging approaches is fine and should be welcomed, but it’s key to remember that we’re dealing with other people and things should not get personal.
How we can improve code reviews for one and all?
In my experience, the best code reviews begin by agreeing on how they will work and what they should look like. As a quick guide, here’s how to get the best from the review process:
Agree on a set of guidelines on how your company or team will introduce and carry out code reviews
Similar to the above, decide how the process can work for you and what you as a team want to get out of it. Decide on some basics such as:
- How to conduct the reviews
- How long they should last; 10 minutes, 20 minutes, etc.?
- When they should happen — e.g. regularly during development, before merge requests, only on a Friday?
- How to handle conflicts or disagreements
During reviews, keep things short and to the point
Like all meetings, code reviews have the ability to go off topic and take far longer than is necessary. Stick to the point: review the code presented to you. Any scope for improvements should be agreed upon and either carried out by the original developer (for smaller fixes or changes) or brought back up as a larger topic for discussion at the next planning meeting.
Don’t involve more people than is necessary, but look for variety and share the knowledge
Ideally, a review should involve the person who made the code and one other reviewer. Contrary to popular belief, the other person doesn’t always have to be a senior or lead developer. Variety is important and will help spread knowledge amongst the team.
It’s worth noting that the reviewer should be at a level that’s comfortable with the code being reviewed. For example, a very junior developer reviewing senior-level code might not be able to contribute in the same way as a more senior level developer. They shouldn’t be discluded but perhaps then is an appropriate time to include a third person.
Keep things positive; focus on the improvements
Try to avoid personal feelings and biases and focus on the code in hand. Suggest improvements where prior experiences can win out or where objective examples — such as a company style guide — can be used.
Another pro tip: actively try to find something positive to talk about. People respond very well to encouragement and like to know they’re doing a good job. There’s always something positive that can be found and it’s a helpful tool to balance out any critical feedback.
Try splitting up reviews into testing and meeting phases
If a body of the code involves quite a few files or large changes, it’s useful to test the code first and have a review meeting later. As the reviewer, this could be when you have a few spare moments or need a break from your own work.
In a previous role, we would carry out reviews via pull request conversations in GitLab. This was often done asynchronously and feedback was provided in comments with examples and diagrams/screenshots given where appropriate.
If you currently take part in code reviews at your company, how do you work with them? Do you like them, are they useful? If not, why not? What’s stopping you?
Good code reviews offer a lot of benefits including improved code quality, better team communication, and knowledge sharing. They’re often avoided or done badly because they involve too much time, negative attitudes, or don’t have a solid structure to build on.