Best Practice For Code Reviews In Engineering Teams

GitHub Code Review Best Practices

Code review is one of the most important practices engineers should have. Your code will always need to be reviewed, and you will always need to review your coworkers' code. If you approach code reviews as a learning process, everyone wins.

Why is Code Review Important?

Perfect for Ensuring Best Practices and Educating Other Developers

Code reviews are one point of contact between engineers during the development process. They are the perfect time to ensure that the team is in sync regarding standards, best practices, and code style.

When done properly, the team will advocate for best practices and share knowledge to raise the average level of the engineering squad. That way, Junior developers will be able to learn quickly and reach their maximum potential with peer feedback.

Helps to Find Defects and Opportunities For Improvement

Code reviews ensure that:

• the test code is robust and valid,

• the design and implementation are consistent across the application.

  
  

They are not about finding bugs, that's what tests are for.

Principles of a Good Code Review

What is not written, does not exist

As Google says, “technical facts and data overrule opinions and personal preferences.” (Google principles for code reviews). As a reviewer, if you request changes on the current implementation, provide resources and explain the benefit of your solution with specific pieces of knowledge. If no resources are found about a specific topic, reach approval and create some documentation for the next time this conflict appears.

Keep the Team Spirit and Be Kind

You are reviewing a teammate or are being reviewed by one. The goal of a code review is to share knowledge, not to hurt the ego. If you are reviewing or responding, think before sending a derogatory comment.

Implement a Code Review Process or Ask For a Review

Most productive engineering teams have a detailed process for code reviews. In this document, explain who you should be asking for a review (is it in a squad? depending on the updated code?), how many approvals do you need (generally from one to three), how to create tests before a code review, and which checks are mandatory to pass before merging the pull request.

If your team does not have a detailed process yet, it is ok to ask for a review. Ping the people you think should review your code, do not wait for someone to find it by themselves.

Review Daily and Sessions Should Last Less Than One Hour

Code reviewing should be a daily habit. As our focus decreases over time, it is best to review code in short sessions and often.

Understand What You're Reviewing First

You won't be able to provide an effective code review if you do not understand what you're looking at.

Having a brief checklist before looking at the code is a great way to be sure that that the code is ready to be reviewed. Inspired by Hugo Dias in his code review article, here is a template checklist that we advise you to customize and follow:

• Does the PR have enough tests to cover the update?
• Does this change require integration tests?
• Is documentation necessary? If yes, is the new documentation (readme, wiki page, or external documentation) online/ready to be online?
• Are the product requirements standards met?
• Does this change update any library or dependency that may break the system?
• Is there any extra task the developer needs to do after merging this code?
• Should someone else review the PR before my review? (ex: security team to validate the changes)

If an item is checked, you should ask for more information before digging into the code.

Test the Code Before Looking at It

You don't want to be the reviewer that approved a pull request where the code is not working. Approving without testing is hazardous. No matter the number of lines changed, you should test the code.

Code Reviews Should be Fast

According to Google code review guidelines about speed, a slow code review process has three negative consequences.

• The product development process slows down. Not reviewing someone's code might fasten your work, but you're slowing down the rest of the team.

• The team starts to protest the code review process. Waiting days for someone to review a few lines of code is frustrating, especially if the review ends by requesting substantial changes.

• Code health is deteriorating. It's easier to accept refactoring or cleanups when you are not yet working on another issue. Fast code reviews raise the code health level.

  
  

If you receive a code review notification and are not working on a focused task, try to review the code as soon as possible. If you do not have time during your day to review the code, do it first thing the next morning.

One Pull Request Should Focus on Only One Task

Separating large tasks into small pull requests helps the team gain in velocity for two reasons:

• Small pull requests are easier to review, so reviewed faster.

• Small changes to the codebase are easier to dive into and understand. So code reviews are more effective.

  
  

The team behind Accelerate: The Science Behind Devops noticed that most efficient engineering teams were working on independent and short-lived branches. That way, every engineer focuses on only one subject at a time, and code reviews are more manageable.

Furthermore, in Best Practices for Peer Code Review, SmartBear conducted a study and found out that developers should not review more than 400 lines of code at a time. Beyond 400 lines, the ability to identify defects diminishes as our brain can not process so much information.

So next time your pull request has more than 400 lines changed, try to separate it into two different pull requests! The best practice is to open stacked pull requests. They are feature branches that are checked out from other feature branches to build small and coherent units to represent changes.

Annotate Your Pull Request and Code Before The Review

A good pull request provides content for the reviewer. If the author of the pull request wrote some comments on the code in tricky parts, it will help the reviewers and prevent avoidable questions from being asked. Provide screenshots, write comments and help your reviewer better understand your work before requesting a review.

Use a Pull Request Template (with a checklist)

A good pull request template will help you better prepare for the review. They are useful to avoid common mistakes, provide clear expectations on what should be ready when requesting a review, and avoid useless back-and-forth during the code review. A checklist inside your pull request template is usually the easiest way to prepare your pull request. You should update your template along the way to customize it to your needs.

Is Styling Important?

If styling is important for your team, you should create a style guide. As we said in our first post, what is not written does not exist. Develop your style guide with the approval of the whole team and what is not inside it, will be a matter of personal preferences.

Here are our internal rules! What do you think?

First Published here