Code Review Best Practices
Code review is a systematic examination of computer source code. It is intended to find mistakes overlooked in the initial development phase, improving the overall quality of software.
1. For the Author (Before Submitting)
- Keep it Small: Large Pull Requests (PRs) are hard to review. Aim for < 400 lines of code. If a feature is big, break it down.
- Self-Review: Read through your own diff before assigning reviewers. You'll often catch silly mistakes (typos, debug prints) yourself.
- Context is King: Write a clear PR description. Explain what changed, why it changed, and how to test it. Screenshots or GIFs are helpful for UI changes.
- Pass CI First: Don't ask for a review if the build is failing or tests are broken.
2. For the Reviewer
- Be Respectful: Critique the code, not the person. Use "we" instead of "you" (e.g., "We should handle this error" vs "You forgot to handle this error").
- Ask, Don't Command: Phrasing feedback as questions often leads to better discussions.
- Bad: "Move this variable."
- Good: "Would it be cleaner if we moved this variable to the top scope?"
- Nitpicks vs. Blockers: Clearly distinguish between critical issues (bugs, security flaws) and personal preferences (variable naming, style).
- Use prefixes like
[NIT](Nitpick) or[OPTIONAL]to indicate non-blocking comments.
- Use prefixes like
3. What to Look For
When reviewing code, check for these key areas:
- Correctness: Does the code actually do what it's supposed to do? Does it handle edge cases?
- Readability: Is the code easy to understand? Are variable names descriptive?
- Security: Are there any obvious vulnerabilities (SQL injection, XSS, exposed secrets)?
- Testing: Are there unit tests covering the new logic? Do the tests actually test the right things?
- Architecture: Does this change fit into the overall design of the system?
4. The "LGTM" Culture
"LGTM" stands for Looks Good To Me. While it's the standard sign-off, avoid rubber-stamping. If you don't understand a piece of code, ask for clarification before approving.
programming/git-version-control programming/ci-cd-pipelines programming/software-testing-basics