What Is Code Review
Google Code Review’s description
The biggest thing that makes Google’s code so good is simple: code review. At Google, no code, for any product, for any project, gets checked in until it gets a positive review.
The reason Google’s code is so good is simple: code review. At Google, no code for any product or project gets merged without review.
It is a culture, not a policy.
Why It’s Needed
Pain Points at Work
- Everyone’s code style is inconsistent
- Interfaces, classes, and names are inconsistent
- Deep logic problems are hard to catch via tests
- Complaints like “Who designed this? So many traps (maybe it was me months ago)”
What It Brings
- Better code quality
- Team knowledge sharing
- Unified team standards
- A shared culture of good code
What to Review
Google Review Principles
What to look for in a code review
- 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.?
- Comments: Are the comments clear and useful?
- Style: Does the code follow our style guides?
- Documentation: Did the developer also update relevant documentation?
Our Review Principles
Architecture and Design
- S.O.L.I.D
- Single Responsibility Principle (SRP)
- Open/Closed Principle (OCP)
- Liskov Substitution Principle (LSP)
- Interface Segregation Principle (ISP)
- Dependency Inversion Principle (DIP)
- Consistent behavior
- Is caching consistent? Is error handling consistent? Are error messages consistent?
- Do the same logic/behavior go through the same code path? Another symptom of low-quality code is that the same behavior/logic appears in different places or is triggered in different ways, without a unified code path, or with copy-pasted implementations everywhere, making maintenance very hard.
- DRY principle (Don’t repeat yourself)
- Program to interfaces, not implementations
- Robustness
- Are corner cases fully considered? Is the logic robust? Any potential bugs?
- Any memory leaks? Any circular dependencies (language-specific, e.g., Objective‑C)? Any wild pointers?
- Thread safety? Data access consistency?
- Error handling
- Good error handling? e.g., network errors, IO errors
- Efficiency / performance
- For frequent messages or large data, are time‑consuming operations handled properly?
- What’s the time complexity of critical algorithms? Any potential performance bottlenecks?
Style
- Engineering conventions (project structure, layering, naming, etc.)
- Apollo configuration conventions
- Naming conventions (interfaces, classes, methods, variables, etc.)
- Code style (braces, spaces, line breaks, indentation)
- Comment conventions (required comments)
- Logging conventions (log what’s necessary)
- DB SQL conventions
- URL conventions
How to Do Code Review
Team Prerequisites
- Git Branch Model
- Git commit messages (should clearly describe changes)
When to Trigger
Tools
- GitLab
- SonarQube