Code Review Guide

A Code Review, or “Peer Code Review,” is a process involving developers looking over code for mistakes, bugs, or other problems. Usually, code reviews involve different developers working together. The primary purpose of code review is to make sure that the overall code health of the code base is improving over time.

The Standard of Code Review

  • Reviewers should favor approving a PR when it definitely improves the overall code health
  • Rreviewer can certainly deny approval even if the code is well-designed (if not appropriate changes)
  • A key point here is that there is no such thing as “perfect” code—there is only better code
  • A PR that improves maintainability, readability, and understandability of the system shouldn’t be delayed for days or weeks because it isn’t “perfect.”
  • Reviewers should always feel free to leave comments expressing that something could be better, but if it’s not very important, prefix it with something like “Nit

Principals

  • Technical facts and data overrule opinions and personal preferences.
  • the style guide is the absolute authority.
  • When there are a few valid options, If the author can demonstrate (either through data or based on solid engineering principles) that several approaches are equally valid, then the reviewer should accept the preference of the author. Otherwise, the choice is dictated by standard principles of software design.
  • If no other rule applies, then the reviewer may ask the author to be consistent with what is in the current codebase, as long as that doesn’t worsen the overall code health of the system.

Don’t let a PR sit around because the author and the reviewer can’t come to an agreement, ask for help (ask the team lead or the engineering manager to help out)

What to look for in a code review

  • Design: The code is well-designed
  • Functionality: The functionality is good for the users of the code, UI changes are sensible and look good, and any parallel programming is done safely.
  • Code Complexity: The code isn’t more complex than it needs to be
  • Code Consistency: is the code consistent with the rest of the codebase?
  • Comments: Comments are clear and useful, and mostly explain why instead of what.
  • Every Line: look at every line of code that you have been assigned to review
  • Tests: Code has appropriate unit tests, integrations tests and tests are well-designed
  • Naming and Style: The developer used clear names for everything, code follow the style guides
  • Documentation: Code is appropriately documented (e.g. README, Confluence pages)

Navigating a PR

  • Always be kind and respectful when making a PR comment
  • Try to respond faster to comments, provide guidance
  • Always provide good explanations and be open to accepting explanations

Related Read:

Leave a Comment