The most surprising thing about code reviews is that their primary goal isn’t finding bugs.
Let’s see a real pull request in action. Imagine this change:
--- a/src/utils.js
+++ b/src/utils.js
@@ -1,5 +1,5 @@
export function formatCurrency(amount, currency = 'USD') {
- return new Intl.NumberFormat('en-US', { style: 'currency', currency }).format(amount);
+ return new Intl.NumberFormat('en-US', { style: 'currency', currency, minimumFractionDigits: 2 }).format(amount);
}
This looks minor, right? Just adding minimumFractionDigits: 2 to a currency formatting function. But what if this function is used in a critical financial dashboard displaying real-time stock prices? Before this change, a price like $10 might appear as $10.0000000000000001 due to floating-point inaccuracies. After the change, it correctly displays as $10.00. A reviewer might catch this by asking: "Where is formatCurrency used, and what are the implications of its output precision in those contexts?"
The real problem code reviews solve is knowledge diffusion and shared ownership. When one person writes code, only they truly understand its intricacies. A review forces them to articulate their choices and exposes the code to other minds, spreading understanding and reducing the blast radius if that original author leaves or is unavailable. It’s about building a collective understanding of the codebase, not just a gate for correctness.
Here’s how to make them effective on GitHub:
1. Small, Focused Pull Requests: Giant PRs are review bottlenecks. Aim for PRs that address a single, well-defined task or bug. A good heuristic is that a reviewer should be able to understand the entire change in 15-30 minutes. If it’s larger, break it down. This makes it easier to spot logical flaws and reduces reviewer fatigue.
2. Clear Descriptions and Context: The PR description is your argument for merging.
- What: Briefly state the problem or feature.
- Why: Explain the motivation behind the change. Link to relevant issues (e.g.,
Fixes #123,Relates to #456). - How: Summarize the approach taken. Highlight any significant design decisions or trade-offs.
- Testing: Describe how you’ve tested the change. Include steps for reviewers to verify it locally if applicable.
3. Use GitHub’s Review Tools Effectively:
- Reviewers: Assign specific individuals who have context on the code being changed. Don’t just request reviews from a generic team.
- Labels: Use labels like
bug,feature,refactor,needs-reviewto categorize PRs and help manage workflows. - Draft PRs: For work in progress, use the "Mark as draft" feature. This signals that the PR is not ready for final review and prevents accidental merging.
- Suggested Changes: For minor tweaks, use the "Suggest changes" feature. This allows reviewers to propose specific line modifications that the author can then "Apply suggestion" with a single click, streamlining the feedback loop.
4. Establish Reviewer Guidelines: Define what you expect from reviewers. This might include:
- Timeliness: Aim to review within 24 business hours.
- Constructive Feedback: Focus on the code, not the author. Frame comments as questions or suggestions.
- Scope: Review for correctness, readability, maintainability, and adherence to project standards. Don’t nitpick on formatting if you have linters.
- Approval Criteria: Clearly state what constitutes an approval (e.g., no blocking comments, all mandatory checks passing).
5. Automate What You Can: Leverage GitHub Actions or other CI/CD tools for:
- Linting and Formatting: Tools like ESLint, Prettier, or Black catch style issues automatically.
- Unit Tests: Ensure basic functionality remains intact.
- Type Checking: For languages like TypeScript, verify type safety.
- Security Scans: Tools like Snyk or Dependabot can identify vulnerable dependencies.
These automated checks should pass before a human reviewer even looks at the code.
6. The "LGTM" Trap and Beyond:
"Looks Good To Me" (LGTM) is often a quick way to approve, but it can be a hollow victory. Encourage reviewers to provide specific feedback. Instead of "LGTM," try: "LGTM, especially the way you handled the edge case in processData() by adding the null check. I also noticed the variable temp_result could be renamed to something more descriptive like processedResults for clarity." This provides actual value.
One thing most people don’t realize is the subtle but powerful effect of asynchronous communication in PRs. Unlike a face-to-face conversation where misunderstandings can escalate quickly, the written nature of PR comments allows for careful phrasing, reflection, and the opportunity for the author to research and respond thoughtfully. This can lead to more considered solutions and a more inclusive environment, especially for remote or neurodivergent team members. It’s a different kind of collaboration, one that trades immediacy for precision and documentation.
After mastering code reviews, the next logical step is understanding how to integrate this process into a broader development workflow, such as Git branching strategies like Gitflow or trunk-based development.