Contents

The “how” part of this post about code reviews got rather long, so I had to split it into two parts. This part is about preparing our code and our team for the review.

This post is part of a series of posts about code reviews:

Team commitment

Code reviews are seen critically by some developers who feel that it’s all a process of criticizing each other among colleagues. It is very important to make that clear to everyone that that is not the case. As described in the first post, we do code reviews to ensure the quality of our code base and learn from each other. However, it is still is not easy for everyone to admit that they might write code that needs a second pair of eyes and, in some cases, improvement.

There is no substitute for really convincing every team member of the benefits and necessity of code reviews. Just making it a mandatory part of the process is not an option. I have seen developers fake the review process: Developers who did not like the code reviews would not give their tasks to be reviewed by colleagues who would actually do an honest review. Instead, they would assign the review to others who would just wave them through or make one minor comment to pretend they had actually looked at the code.

Management needs to be committed to the review process as well. If they are not, they may see it as a time waster that will be the first thing to erase without replacement when time pressure arises. As with other quality gates, code reviews might be shortened when there really is no time, but it must be clear that this means to accumulate technical debt similar to not refactoring or writing automated tests.

Make your code review-ready

Giving a piece of code to a reviewer is basically a tiny shipment of that code to the team. For many, there is nothing more embarrassing than shipping something incomplete, unusable or broken. It also is a huge waste of time for both the reviewer and the reviewee if meta-discussions have to take place before the actual review. That’s why it’s a good idea to take a step back and have a second look at the code we’re about to commit to the code review.

Make sure the code is “Done”

If code reviews are part of your team’s development process, your Definition of Done probably contains the code review itself, i.e. your code is not “Done” unless it has passed the review. However, the code review should be among the last points of the DoD, if not the very last. Before you open that pull request or ask for a reviewer in your team’s Slack or whatever starts the review process, make sure that all the other points are done.

For example, does the code compile, are the tests written and do the pass? Is everything checked in, and does all that also work on another machine? There are nice tool integrations for CI servers and version control systems, that can give you an automated answer to many of these questions. It can also be a good idea to make a checklist that reminds you to check these points whenever you open a new pull request. You can find a more detailed article about code review checklists here.

Don’t submit giant PRs

Imagine you are supposed to do a code review on a full month’s work with dozens of new files and hundreds or even thousands of edits. Overwhelming, right? It will take some time to review it all, and since attention spans are not that long, a thing or two will slip through that we would have found otherwise.

If we do a larger chunk of work, there is much benefit in splitting it into smaller steps, not only for the code reviews. Giving bite-sized pieces of that work to the reviewer has many benefits. We get feedback faster and also earlier in the process, so we can correct mistakes or factor in good suggestions before everything is set in stone.

I’ve gotten the question what to do when we need the code we just submitted for review for our next steps and can’t afford to wait for the review to be completed and the code to be merged. If you are using git or a similar VCS, an option is to start a new branch on the commit you submitted to review. While that may not strictly be Git-Flow or GitHub-Flow or whatever branching model you use, you can always rebase the new branch once the old one has been merged.

Another option is to make sub-feature branches that get merged into a larger feature branch. Use the possibilities of your tooling and adapt the version control process to your needs. After all, we are not (and should not be) slaves of our tools and processes.

Provide some context

It can be frustrating for a reviewer to have a pile of code dumped on your doorstep for review without any explanation what the code you’re about to review is supposed to be doing. It might be tempting for the reviewee to just sit down with the reviewer and explain everything that is needed while going through the code. However, for reasons I’ll explain in the next post, it is best if the reviewer approaches the code on their own at first.

Therefore it is important to provide the reviewer with some context. Good commit messages are a good start, as tools use them often for the pull request summary. Another option is a link to the ticket that is solved by the code, and a hint which files to review first (e.g. the documentation or the central test suite of a new feature) never hurts.

Of course, we want our code to be reviewed, not our ability to write prose on the pull request summary, so the summary should still be short. That does, however, not mean that it should be shorter than needed – it may even include images if your tooling allows it.

Conclusion

There’s a lot to think of before we actually start the code review itself. However, most of these things should be done one way or another anyway. It’s a good thing to have them sorted out before we submit our pull request since it only makes life easier for the reviewer.

In the next post, I’ll go into how the code review itself can be done.

All emojis by emojione.com

Write clean and maintainable C++

Code Reviews – Preparation (Part 3)