👓Google's conduct of code reviews: Part 1 – Two sides of the same coin 🪙

Code reviews can be a daunting task, or a breeze to go through.. or anything in between. There are small things that we can do bit by bit to enable us, as code reviewers and authors of the code, to fully engage in code reviews without fear. With the right steps taken from both sides we might even enjoy it!

This blog will briefly go into the world of code reviews through the eyes of Google’s code review best practices here, with the aim of shedding light on what we can do as devs to improve our code review process. We will go through some of the main sections, highlighting information which could be most beneficial in improving the process of code reviews.

For part 1, we’ll get some things straight on PRs, briefly talk about the mix of personalities with experience levels and how it could affect it. We will also go through the overall standards of conducting code reviews, which affects both sides of this coin.

Let’s get some things straight

A code review should not be viewed as a routine to click the “go” button on without really looking, and it’s not unchangeable (this is for the ones who think their code is always right). Although these points are very obvious, it can easily slip unnoticed from time to time if no one opens this discussion in their company. “Well... it looks good to me from what I’ve skimmed so I’ll approve quickly and get back to my thing”. Don’t lie, we’ve all been there in the earlier days and we’re all human.

Senior vs Junior, easy going vs hard headed

Throughout my first year as a dev, it never really occurred to me to really, genuinely, look at the code during code reviews. I was actually judging it by looking at the person who wrote the code and accepted it because, well, they knew more than me so I didn’t want to say anything. That horrible habit grew unnoticed until I met someone who reviewed my code with such detail, every time, that it kicked “code reviews” right into the forefront of my learning list (no one was harmed during the kicking, it all happened in my head 🧠).

The personality/experience of the change author/code reviewer is just as much of a culprit in creating an environment where everything is a green/red button in PRs. Being too easy going or too hard headed serves no purpose in PRs, it will either create more bugs or delivery will slow down. We need to talk about where personality as well as experience come into play during PRs : to help juniors and seniors navigate, reflecting on their performance in them and with as much profession as possible.

The Standard of Code Review - Link here

This part is from the code reviewer’s section; however, some parts apply to both sides. Specifically, the main aim of conducting a PR is ensure that the overall health/quality of the codebase increases over time. This aim ensures the “why” part for both the reviewer and the change author, so it should (hopefully) quieten down the variable of personality within a code review.

A key point to also mention is that no code is ever perfect, so it is encouraged that as long as the change code is better in quality than the current codebase then it should be alright to approve. This is a direct reflection to the following rule mentioned:

“In general, reviewers should favor approving a CL once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn’t perfect.”

PRs can serve as a function to improve developers' standards of programming, as well as gain an opportunity to learn more about the language/framework their PR is in. A bit of a warning, too much of it can serve as hindrance in the view of the change author, they’re probably thinking “it’s not important, just approve already!”. As for the author, having a learning state of mind during PRs will always give you something out of it afterwards: you also don’t want to create the same erroneous code again and again in the upcoming PRs.

“Just keep in mind that if your comment is purely educational, but not critical to meeting the standards described in this document, prefix it with “Nit: “ or otherwise indicate that it’s not mandatory for the author to resolve it in this CL.”

There is also the principles subsection which states four principles to be guided by when in communication during the code review.

• Technical facts and data overrule opinions and personal preferences
• On matters of style, the style guide is the absolute authority
• Aspects of software design are based on underlying principles and should be weighed on those principles, not simply by personal opinion
• If none of the above applies, look at what is the most consistent in the codebase, as long as it doesn’t decrease codebase health/quality

When in communication during the code review, there will most likely be differences of opinions. That is A OK, there’s no reason to shy away from that possibility. If both opinions are valid then meeting in the middle is the first prize in this situation. Otherwise, a face-to-face conversation or even including a senior figure that can weigh in would be good too.

Code reviews are sometimes small chances to witness moments of care that a person shows to a code throughout the day whenever we see it pop up in our notifications. It’s small moments like these that help us reflect on how we interact with others, as well as show how much care we are willing to show the codebase: whether it’s ours or someone else’s. So why not try and make those small moments better, as every drop makes an ocean 💧

As we walk along Google’s code review best practices, there are two sides to a code review which we will look at in the upcoming parts of this series. These are the code reviewer’s side and the change author’s side. Each section has subsections which we will summarise for convenience and highlight the most important bits to get us up and running in improving our PR flow. Thank you for your time, and see you then! 👓