Many developers start their careers without really knowing what code reviews are for. Whether we studied at university or joined a coding bootcamp, the focus is usually on learning algorithms and writing code that “works”, while quality and collaboration are seldom covered.

It was the same for me when I was at uni. I was barely taught about version control systems, and as I worked on personal projects and assignments, I learnt about the value of version control systems as a way to track my changes, but not as a way to improve my code and collaborate with others.

It was only after joining the workforce that I started to understand the value of code reviews. It’s something I’ve learnt and improved over the years by working with different teams. In this article I’ll share why code reviews are important, and how to do them effectively in a software development team. There will be a second part where I’ll cover how to effectively use code reviews as a communication and collaboration tool.

Code Reviews Matter

When code reviews are not done properly, things start to break down, both in the codebase and in the team.

Code quality drops if we follow our own patterns and we’re not aligned with the team. Bugs become more frequent and harder to spot the more we code.

We stop learning from our peers, missing lots of chances to get better at coding. In the end, that means we’re not growing professionally. This is not science fiction: this has happened and is still a problem for many people in many teams.

We may also be seen as arrogant or snobs. if our team gives us feedback and we just dismiss it, or when we may have something to contribute but we keep it to ourselves.

The way to avoid all this is to remember to be team players when we participate in code reviews, not superheroes. Companies need people who can collaborate and work together towards shared goals.

In this article, I will cover two of four key strategies that we can follow to be effective at code reviews: prepare the code, and make pull requests easy to review. I’ll discuss the other two in the next article.

Prepare The Code

Before even thinking of raising a pull request we need to believe, to the best of our abilities, that our code is production ready. This means having code that meets the (hopefully) high-quality standards of our team, and to make sure that it actually works.

To ensure that the quality is right, I like to follow the advice in the Clean Code and Clean Architecture books by Robert C. Martin:

First make it work. Then make it right.

Maybe this is not too important for small prototypes or university assignments, but when we’re building a large commercial product with a team, this is a must.

Sadly, I’ve seen many engineers entirely skipping the second step, because it’s easier, and doing the right thing normally requires an extra effort. In my opinion, this is one of the main causes why code deteriorates over time. When we all focus on delivering high quality code, and we make each other accountable for this, the quality of the final product is much higher.

It may seem unnecessary to remind us to test our code before raising a pull request, but there are still many people who just don’t do that. Even very experienced people do that sometimes, out of an excess of well earned confidence. However, we’ll always make mistakes because we are human beings. Based on my experience, the best way to prevent some silly bugs is to actually test our code.

One of the most humiliating things that can happen to me is when someone finds a bug in my code that I could have easily caught if I had just run it once. I don’t mean to disrespect my teammates, but they’re spending their time reviewing my work. So if I can’t even do the basics, I would not blame them for feeling that way.

There are a few exceptions to this though. If we just want to write a prototype or proof of concept, this doesn’t have to be good at all. Sometimes we need to merge these prototypes into a repo to actually try it out, and as long as it kind of runs and we can prove our hypothesis, that’s fine.

Another good exception I consider is when we have absolutely no clue if what we are doing is the right thing. Raising a PR to get feedback on the approach that we are following is a super useful technique, as long as we clearly communicate our intent and that the work-in-progress code is not meant to be merged, yet.

Bitbucket and GitHub have already introduced a feature to raise draft pull requests, to make it even more prominent. I’ve noticed that sometimes people completely miss these flags in pull requests, so even if you decide to do that, make sure that your teammates understand that the intent of raising the pull request.

Once we confirm or rule this out, we should keep working on improving our solution or starting it all over again if needed.

Make Pull Requests Easy to Review

Pull Requests are a core part of how we do code reviews today, but we often don’t use them as effectively as we could. In my article Effective Pull Requests for Better Code Reviews, I explain what makes a good Pull Request and how to make them easier to review. This includes:

• Adding useful context like screenshots, videos or comments
• Keeping PRs small and focused
• Writing clear descriptions
• Using tools like Git to break down work when needed

In this article, I’d like to expand on a few things that I wish I had covered in that article:

Using before and after screenshots

I find it quite frustrating when I need to review a frontend pull request, and I can’t tell what the change is visually. Even if I can identify the elements in it, I am not a machine, so I can’t process code into a correct visual representation of them.

For example, once I made a frontend change and added a before and an after screenshot to the pull request. A teammate looked at the screenshot and noticed that a pattern had shifted by one pixel. I didn’t believe it at first, because I had not added done any changes in positioning. After carefully checking, we could confirm that he was right, and that the issue was caused when I used a specific React component to wrap another component. Although this was not a mission critical issue, I think it highlights the usefulness of allowing reviewers to visually compare user interface changes.

Keeping PRs small and focused

I’ve seen many times that, because pull requests are too large, people just avoid reviewing them. And if they review them, there are many bugs or issues that are not detected.

I have confirmed this many times. For example, when I include a change similar to that of another teammate, only to get a comment in my pull request and not in theirs. The only difference between our pull requests is that mine was small and easy to review, so the issue was evident. The lesson here is not to send large pull requests so we can merge our code quickly, but to send smaller pull requests so we can identify and amend issues.

I sometimes end up with large chunks of code when working on some changes. This usually happens to me when I have not identified all the dependencies of my changes, so I need to change a lot of related code that I was not aware, or that just slipped my mind.

In those cases, I keep working until I identify all dependencies for my change and I have it working end-to-end, and then I split my work into smaller parts.

I can easily do this because I have mastered some powerful but underused git commands, that allow me to raise a pull request with only a subset of my changes, while safely keeping the rest in a safe place, ready to have a new pull request when that is merged.

If there is one technology that we should definitely master, that is our version control client.

Final Thoughts

Before we ask someone to review our code, we should make it as good as we can. That saves time, builds trust, and shows respect for others.

In the next article, I’ll show how we can use code review to better communicate, collaborate and learn from each other.

Cheers!
José Miguel

Share if you find this content useful, and Follow me on LinkedIn to be notified of new articles.