Code Review Process | Quality + Knowledge Sharing
Establish code review practices with GitScrum's GitHub, GitLab, Bitbucket integrations. Track PR status on tasks, improve quality, and spread knowledge across the team.
17 min read
Code review is more than bug catchingβit's a quality gate, knowledge sharing mechanism, and team alignment tool. Effective review processes balance thoroughness with speed, ensuring high standards without becoming a bottleneck. GitScrum's Git integrations connect reviews to your project workflow, maintaining visibility and accountability across the development process.
Code Review Fundamentals
Purpose of Code Review
WHY WE REVIEW CODE:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β OBJECTIVES BEYOND BUG FINDING β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β PRIMARY OBJECTIVES: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β ββ
β β π DEFECT DETECTION ββ
β β Catch bugs before they reach production ββ
β β Logic errors, edge cases, security issues ββ
β β ββ
β β π KNOWLEDGE SHARING ββ
β β Spread understanding across the team ββ
β β Reduce knowledge silos, bus factor ββ
β β ββ
β β π STANDARD ENFORCEMENT ββ
β β Maintain consistent code patterns ββ
β β Architecture alignment, style consistency ββ
β β ββ
β β π MENTORING ββ
β β Teach and learn through review dialogue ββ
β β Junior development, senior oversight ββ
β β ββ
β β π€ COLLECTIVE OWNERSHIP ββ
β β Multiple people understand each change ββ
β β Better maintenance, faster debugging ββ
β β ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β REVIEW IS NOT: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β Quality assurance replacement (need testing too) ββ
β β β Performance evaluation (separate process) ββ
β β β Architectural design time (too late for big changes) ββ
β β β Style enforcement (use automated tools) ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
What to Review
REVIEW FOCUS AREAS:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β PRIORITIZING REVIEW ATTENTION β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β HIGH PRIORITY (Always check): β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β ββ
β β β CORRECTNESS ββ
β β Does it do what it's supposed to? ββ
β β Edge cases handled? ββ
β β Error scenarios covered? ββ
β β ββ
β β β SECURITY ββ
β β SQL injection, XSS vulnerabilities? ββ
β β Authentication/authorization correct? ββ
β β Sensitive data handling appropriate? ββ
β β ββ
β β β DESIGN ββ
β β Does it fit the architecture? ββ
β β Appropriate abstraction level? ββ
β β Maintainable and extensible? ββ
β β ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β MEDIUM PRIORITY (Usually check): β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β ββ
β β β TESTABILITY ββ
β β Are there adequate tests? ββ
β β Tests cover the right cases? ββ
β β Easy to test changes? ββ
β β ββ
β β β PERFORMANCE ββ
β β Any obvious inefficiencies? ββ
β β Database query patterns? ββ
β β Memory usage concerns? ββ
β β ββ
β β β DOCUMENTATION ββ
β β Complex logic explained? ββ
β β Public APIs documented? ββ
β β README updated if needed? ββ
β β ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β LOWER PRIORITY (Automate if possible): β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β ββ
β β β³ Formatting and style β Use linters/formatters ββ
β β β³ Import organization β Automate with tools ββ
β β β³ Naming conventions β Define rules, enforce with CI ββ
β β β³ Code structure β Use static analysis ββ
β β ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
Integrating with GitScrum
PR Visibility
GITSCRUM GIT INTEGRATION:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β CONNECTING PULL REQUESTS TO TASKS β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β AUTOMATIC LINKING: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β When developers include task ID in branch/PR: ββ
β β ββ
β β Branch naming: ββ
β β feature/PROJ-123-user-authentication ββ
β β ββ
β β PR title: ββ
β β PROJ-123: Add user authentication flow ββ
β β ββ
β β GitScrum automatically: ββ
β β βββ Links PR to task PROJ-123 ββ
β β βββ Shows PR status on task card ββ
β β βββ Updates activity feed ββ
β β βββ Notifies assignees of PR events ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β TASK VIEW WITH PR: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β βββββββββββββββββββββββββββββββββββββββββββββββββββββββ ββ
β β β PROJ-123: User Authentication β ββ
β β β Status: In Review β ββ
β β β β ββ
β β β Linked Pull Requests: β ββ
β β β βββββββββββββββββββββββββββββββββββββββββββββββββββ β ββ
β β β β π‘ PR #456 - Add user authentication flow β ββ
β β β β Status: Changes requested β ββ
β β β β Reviews: 1/2 approved β ββ
β β β β CI: β Passing β ββ
β β β β Updated: 2h ago β ββ
β β β βββββββββββββββββββββββββββββββββββββββββββββββββββ β ββ
β β β β ββ
β β β Activity: β ββ
β β β β’ @chen left review with 2 comments (1h ago) β ββ
β β β β’ @sarah pushed 3 commits (3h ago) β ββ
β β β β’ PR opened by @sarah (1d ago) β ββ
β β βββββββββββββββββββββββββββββββββββββββββββββββββββββββ ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β BOARD VISIBILITY: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Code Review column shows PR status: ββ
β β ββ
β β βββββββββββββ βββββββββββββ βββββββββββββ ββ
β β β PROJ-123 β β PROJ-456 β β PROJ-789 β ββ
β β β Auth flow β β Dashboard β β Reports β ββ
β β β β β β β β ββ
β β β π’ PR #456 β β π‘ PR #461 β β π΄ PR #458 β ββ
β β β Approved β β Reviewing β β Changes β ββ
β β βββββββββββββ βββββββββββββ βββββββββββββ ββ
β β ββ
β β Quick filters: [All PRs] [Needs Review] [Approved] ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
Review Workflow
REVIEW PROCESS INTEGRATION:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β END-TO-END REVIEW WORKFLOW β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β 1. TASK STARTED β IN PROGRESS β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Developer: ββ
β β β’ Moves task to "In Progress" ββ
β β β’ Creates branch with task ID ββ
β β β’ Develops and tests locally ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β
β 2. PR CREATED β IN REVIEW β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Developer: ββ
β β β’ Opens PR with task ID in title ββ
β β β’ GitScrum auto-moves task to "Code Review" ββ
β β β’ Requests reviewers ββ
β β ββ
β β Reviewers notified via: ββ
β β β’ GitScrum notifications ββ
β β β’ Slack/Teams integration ββ
β β β’ Email (based on preferences) ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β
β 3. REVIEW CYCLE β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Reviewers: ββ
β β β’ Review code in GitHub/GitLab ββ
β β β’ Leave comments, approve, or request changes ββ
β β ββ
β β Task reflects: ββ
β β β’ Review status (pending/approved/changes requested) ββ
β β β’ CI status (passing/failing) ββ
β β β’ Comment count ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β
β 4. APPROVED β READY TO MERGE β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β All reviewers approved: ββ
β β β’ PR status shows "Approved" ββ
β β β’ Task can move to "Ready for QA" or merge directly ββ
β β β’ Developer or tech lead merges PR ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β
β 5. MERGED β DONE β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β On merge: ββ
β β β’ GitScrum can auto-move task to "Done" ββ
β β β’ Or move to "Deployed" for deployment tracking ββ
β β β’ Activity log updated ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
Review Guidelines
For Authors
SUBMITTING CODE FOR REVIEW:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β SETTING UP REVIEWERS FOR SUCCESS β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β BEFORE REQUESTING REVIEW: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β‘ Self-review your changes first ββ
β β β‘ Run all tests locally (they should pass) ββ
β β β‘ Check linting/formatting passes ββ
β β β‘ Remove debug code, console.log, TODOs ββ
β β β‘ Verify PR is reasonably sized (<400 lines) ββ
β β β‘ Add tests for new functionality ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β PR DESCRIPTION TEMPLATE: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β ## Summary ββ
β β Brief explanation of what this PR does ββ
β β ββ
β β ## Task ββ
β β Closes PROJ-123 ββ
β β ββ
β β ## Changes ββ
β β - Added authentication middleware ββ
β β - Created user session management ββ
β β - Updated API routes for protected endpoints ββ
β β ββ
β β ## Testing ββ
β β - [ ] Unit tests added for AuthService ββ
β β - [ ] Integration tests for login flow ββ
β β - [ ] Manual testing of OAuth flow ββ
β β ββ
β β ## Screenshots (if UI change) ββ
β β [Before/After images] ββ
β β ββ
β β ## Notes for Reviewers ββ
β β Focus on security in AuthMiddleware ββ
β β Session timeout logic in SessionManager ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β SMALL, FOCUSED PRs: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β PR Size β Review Time β Quality ββ
β ββββββββββββββββββΌββββββββββββββββΌβββββββββββββββββββββββββ
β β < 100 lines β 15-30 min β Thorough ββ
β β 100-400 lines β 30-60 min β Good ββ
β β 400-800 lines β 60-90 min β Declining ββ
β β > 800 lines β Often skimmed β Poor (split the PR!) ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β RESPONDING TO FEEDBACK: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β Respond to every comment ββ
β β β Ask for clarification if unclear ββ
β β β Explain reasoning if you disagree ββ
β β β Push fixes as new commits (for traceability) ββ
β β β Re-request review when ready ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
For Reviewers
GIVING EFFECTIVE REVIEWS:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β REVIEW BEST PRACTICES β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β REVIEW MINDSET: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β Review the code, not the person ββ
β β β Assume positive intent ββ
β β β Consider multiple valid approaches ββ
β β β Be explicit about severity (blocker vs suggestion) ββ
β β β Offer alternatives, not just criticism ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β COMMENT TYPES: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β BLOCKING (Must fix): ββ
β β "π΄ BLOCKER: This SQL is vulnerable to injection. ββ
β β Use parameterized queries instead: ββ
β β db.query('SELECT * FROM users WHERE id = ?', [userId])"ββ
β β ββ
β β SUGGESTION (Should consider): ββ
β β "π‘ SUGGESTION: Consider using async/await here ββ
β β for better readability. Not blocking but would ββ
β β improve maintainability." ββ
β β ββ
β β QUESTION (Need clarification): ββ
β β "β QUESTION: Why are we caching this for 24 hours? ββ
β β Is there a specific requirement?" ββ
β β ββ
β β NIT (Minor, optional): ββ
β β "π NIT: Typo in variable name 'recieve' β 'receive' ββ
β β Feel free to ignore if you prefer." ββ
β β ββ
β β PRAISE (Positive feedback): ββ
β β "π Nice approach! This handles the edge cases ββ
β β elegantly." ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β TIME EXPECTATIONS: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Review requests should be handled within: ββ
β β ββ
β β First response: < 4 hours during work hours ββ
β β Full review: < 1 business day ββ
β β Re-review: < 4 hours (smaller changes) ββ
β β ββ
β β If can't review, decline and suggest alternative ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β APPROVAL CRITERIA: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β β‘ Code works correctly (tested or verified) ββ
β β β‘ No security vulnerabilities introduced ββ
β β β‘ Follows team architecture and patterns ββ
β β β‘ Adequate test coverage ββ
β β β‘ Documentation updated if needed ββ
β β β‘ No blocking comments remaining ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
Team Practices
Review Assignments
WHO REVIEWS WHAT:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β REVIEWER ASSIGNMENT STRATEGIES β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β CODE OWNERS: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Define owners by directory/file pattern: ββ
β β ββ
β β # CODEOWNERS file ββ
β β /src/auth/* @security-team ββ
β β /src/api/* @backend-team ββ
β β /src/components/* @frontend-team ββ
β β /infrastructure/* @devops ββ
β β *.md @docs-team ββ
β β ββ
β β Auto-assigns reviewers based on changed files ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β REVIEWER COUNT: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Change Type β Required Reviewers ββ
β βββββββββββββββββββββββββΌββββββββββββββββββββββββββββββββββ
β β Standard feature β 1 peer + 1 senior ββ
β β Security-related β 1 peer + security lead ββ
β β Database changes β 1 peer + DBA/data lead ββ
β β Architecture changes β 2 seniors/architects ββ
β β Hotfix (urgent) β 1 senior (expedited) ββ
β β Documentation only β 1 reviewer ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β ROTATION STRATEGIES: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β ROUND-ROBIN: ββ
β β Distribute review load evenly across team ββ
β β Each person gets similar review count per sprint ββ
β β ββ
β β EXPERTISE-BASED: ββ
β β Match reviewer expertise to change type ββ
β β Database changes β people who know DB well ββ
β β ββ
β β PAIRING: ββ
β β Always include one junior + one senior ββ
β β Knowledge transfer in both directions ββ
β β ββ
β β AUTHOR'S CHOICE + RANDOM: ββ
β β Author picks 1 reviewer they think is best ββ
β β System randomly assigns 1 more for diversity ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
Review Metrics
MEASURING REVIEW HEALTH:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β TRACKING REVIEW PROCESS EFFECTIVENESS β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β KEY METRICS: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Metric β Target β Warning ββ
β βββββββββββββββββββββββΌββββββββββββββββΌββββββββββββββββββββ
β β First response time β < 4 hours β > 8 hours ββ
β β PR cycle time β < 24 hours β > 48 hours ββ
β β Avg review rounds β < 2 rounds β > 3 rounds ββ
β β Comments per PR β 3-8 comments β < 1 or > 15 ββ
β β Review depth β All files β Partial review ββ
β β Approval rate β 80-95% β < 70% or 100% ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β INTERPRETING METRICS: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Very few comments per PR: ββ
β β β Reviews might be too shallow ββ
β β β PRs might be too perfect (unlikely) ββ
β β β Review culture might discourage feedback ββ
β β ββ
β β Too many comments per PR: ββ
β β β PRs too large, should be split ββ
β β β Standards not clear, lots of style comments ββ
β β β Design discussions happening in PR (too late) ββ
β β ββ
β β Long cycle time: ββ
β β β Reviewers overloaded ββ
β β β PRs too large to review quickly ββ
β β β Review not prioritized ββ
β β ββ
β β 100% approval rate: ββ
β β β Rubber-stamping, not real reviews ββ
β β β Should see some iteration ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
Automation
CI Integration
AUTOMATED CHECKS BEFORE REVIEW:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β LET MACHINES CHECK WHAT MACHINES CHECK BEST β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β PRE-REVIEW AUTOMATION: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β CI Pipeline (must pass before review): ββ
β β ββ
β β β‘ Build compiles successfully ββ
β β β‘ All tests pass ββ
β β β‘ Code coverage above threshold ββ
β β β‘ Linting passes (no style issues) ββ
β β β‘ Formatting correct (Prettier, Black, etc.) ββ
β β β‘ Security scan passes (no critical vulnerabilities) ββ
β β β‘ Type checking passes (TypeScript, mypy) ββ
β β ββ
β β If CI fails β Author must fix before requesting review ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β AUTOMATED REVIEW COMMENTS: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Bots can comment on common issues: ββ
β β ββ
β β β’ "PR is too large (1,247 lines). Consider splitting." ββ
β β β’ "Missing test coverage for new function X." ββ
β β β’ "Deprecated API usage detected in file Y." ββ
β β β’ "This file has changed 47 times in 30 days - high ββ
β β change area, consider extra review." ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β GITSCRUM VISIBILITY: β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Task shows CI status: ββ
β β ββ
β β βββββββββββββββββββββββββββββββββββββββββββββββββββββββ ββ
β β β PROJ-123: User Authentication β ββ
β β β β ββ
β β β PR #456 CI Status: β ββ
β β β β Build Passed β ββ
β β β β Tests 142/142 passed β ββ
β β β β Linting No issues β ββ
β β β β Coverage 87% (β₯80% required) β ββ
β β β β Security No vulnerabilities β ββ
β β β β ββ
β β β Ready for review β β ββ
β β βββββββββββββββββββββββββββββββββββββββββββββββββββββββ ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
Common Anti-Patterns
What to Avoid
CODE REVIEW ANTI-PATTERNS:
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β PRACTICES THAT HARM REVIEW EFFECTIVENESS β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ€
β β
β β RUBBER STAMPING β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Problem: Approving without reading ββ
β β Sign: Quick approvals, no comments, bugs in production ββ
β β Fix: Require minimum review time, track comment rate ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β β GATEKEEPING β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Problem: Using review to block PRs for personal style ββ
β β Sign: Same person blocks everything, opinion-based ββ
β β Fix: Clear standards, escalation path, rotate reviewers ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β β NEVERENDING REVIEW β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Problem: New comments after every round ββ
β β Sign: 5+ review rounds, moving goalposts ββ
β β Fix: All feedback in first round, time-box reviews ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β β DRIVE-BY REVIEWS β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Problem: Leaving comments but not approving/blocking ββ
β β Sign: PRs stuck with comments but no decision ββ
β β Fix: Must approve, request changes, or explicitly pass ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β β REVIEWING ONLY HAPPY PATH β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Problem: Only looking at main logic, ignoring edges ββ
β β Sign: Edge case bugs in production ββ
β β Fix: Checklist including error handling, null checks ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
β β DESIGN REVIEW IN CODE REVIEW β
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β Problem: Questioning architecture after code is written ββ
β β Sign: "Why are we doing it this way?" in PR ββ
β β Fix: Design reviews before coding, RFCs for big changes ββ
β ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
β β
βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
Best Practices Summary
Do's
EFFECTIVE CODE REVIEW PRACTICES:
β AUTOMATE STYLE CHECKS
Let linters handle formatting, focus on logic
β KEEP PRs SMALL
<400 lines enables thorough review
β RESPOND QUICKLY
<4 hours first response keeps flow
β BE SPECIFIC IN FEEDBACK
Show the alternative, not just the problem
β LABEL COMMENT SEVERITY
Blocker vs suggestion vs nit
β CELEBRATE GOOD CODE
Praise builds culture
Don'ts
CODE REVIEW PITFALLS:
β REVIEWING GIANT PRs
>800 lines will be skimmed, not reviewed
β STYLE DEBATES IN REVIEWS
Automate with formatters, or document standards
β APPROVING WITHOUT READING
Defeats the purpose entirely
β BEING HARSH
Code review is for code, not person
β IGNORING TESTS
Test quality matters as much as code