Opinionated summary of Google’s Code Review Developer Guide.

Principles

• Be kind - Reviewing includes criticizing the work of others, which is delicate. Empathize, give praise for good things, be constructive and make it clear that any criticism is not directed at the person, but on the code.
• Be fast - A review should be done no later than one business day, because the author can easier fix it and it’s less likely to block other changes.
• Explain your thought - Explain why your are making a comment, what is your intent.

Steps of a review

In case of any finding while going through the following steps, don’t continue with the next step but ask the author for clarification or change instead!

1. Take a broad view of the change - Read the description and/or take a look at corresponding issue.
2. Examine the main part - Look at the file and section which contains the most (important) changes first.
3. Look through the rest - Inspect all other changes done.

What to look for

• Mention good things - If you see something nicely done, mention it to the author!
• Design - Is the code well-designed and appropriate for your system?
• Functionality - Does the code behave as the author likely intended? Is the way the code behaves good for its users?
• Complexity - Could the code be made simpler? Would another developer be able to easily understand and use this code when they come across it in the future?
• Tests - Does the code have correct and well-designed automated tests?
• Naming - Did the developer choose clear names for variables, classes, methods, etc.?
  • Use long descriptive names
  • Avoid ambiguous naming
  • Use the same vocabulary from the code base
  • Avoid redundant context. E.g., in class Person don’t declare self.person_age but just self.age.
• Numbers: - Don’t use magic numbers (unexplained constant numbers)! Instead, create a constant/variable for it with a descriptive name. Example:

  # NO
  dice_roll = random.randint(0, 4)
  # YES
  DICE_SIDES = 4
  dice_roll = random.randint(0, DICE_SIDES)

• Functions
  • Functions should do only one thing only.
  • Do not use boolean flags in arguments:

    # NO
             def transform_text(text: str, uppercase: bool) -> str:
           if uppercase:
                  return text.upper()
              else:
               return text.lower()
    # YES
    def transform_to_upper(text: str) -> str:
    	return text.upper()
    def transform_to_lower(text: str) -> str:
        return text.lower()

• Data Types:
  • Don’t use list if tuple is sufficient.
• Comments - Are the comments clear and useful?
• Style - Does the code follow our style guides?
• Documentation - Did the developer also update relevant documentation?

Getting reviewed

• Identify Reviewer - Find the best person(s) for this task. If not available, notify them on the change.
• Be open - Don’t take any comment personally, try to suppress the natural urge to justify or defend yourself, step back instead and see the comment as a possibility for both of you to learn.