Git & GitHub

Code Review Best Practices

13 min Lesson 27 of 35

Code Review Best Practices

Code reviews are one of the most effective ways to improve code quality, catch bugs early, and share knowledge across teams. In this lesson, we'll explore best practices for both reviewing code and responding to reviews, using GitHub's powerful review features.

Why Code Reviews Matter

Benefits of Code Reviews:

✓ Catch Bugs Early: Find issues before they reach production
✓ Improve Code Quality: Ensure standards and best practices
✓ Knowledge Sharing: Team learns from each other
✓ Consistency: Maintain codebase patterns and style
✓ Documentation: PR discussions become documentation
✓ Mentorship: Junior developers learn from seniors
✓ Collective Ownership: Team shares responsibility for code

Research Shows: Code reviews catch 60-90% of defects before production, making them more cost-effective than finding bugs in production.

What to Look For When Reviewing

A comprehensive code review checklist:

1. Functionality
   ☐ Does the code do what it's supposed to?
   ☐ Are edge cases handled?
   ☐ Are error conditions handled properly?
   ☐ Is the logic correct and efficient?

2. Design & Architecture
   ☐ Is the code in the right place?
   ☐ Does it follow existing patterns?
   ☐ Is it maintainable and extensible?
   ☐ Are responsibilities properly separated?

3. Code Quality
   ☐ Is the code readable and clear?
   ☐ Are variable/function names descriptive?
   ☐ Is there unnecessary complexity?
   ☐ Is code duplicated unnecessarily?

4. Testing
   ☐ Are there sufficient tests?
   ☐ Do tests cover edge cases?
   ☐ Are tests readable and maintainable?
   ☐ Do all tests pass?

5. Security
   ☐ Are there SQL injection vulnerabilities?
   ☐ Is user input properly validated?
   ☐ Are secrets properly protected?
   ☐ Are authentication/authorization correct?

6. Performance
   ☐ Are there N+1 query problems?
   ☐ Are expensive operations optimized?
   ☐ Is caching used appropriately?
   ☐ Are there memory leaks?

7. Documentation
   ☐ Are complex parts commented?
   ☐ Is the PR description clear?
   ☐ Are breaking changes documented?
   ☐ Is documentation updated?

Using GitHub Review Features

GitHub provides powerful tools for effective code reviews:

1. Comment Types:

# Single line comment
- Click line number, add comment
- Use for specific line issues

# Multi-line comment
- Click and drag line numbers
- Use for reviewing code blocks

# General comment
- Use PR conversation tab
- Use for overall feedback

# Suggestion
- Click "Insert a suggestion" icon
- Reviewer proposes exact code change
- Author can commit suggestion directly

Writing Effective Review Comments

How you communicate feedback is as important as what you say:

❌ Bad Comment:
"This is wrong."

✅ Good Comment:
"This could cause a null pointer exception when user is not authenticated.
Consider adding a check:

if (!auth()->check()) {
    return redirect()->route('login');
}

What do you think?"

Best Practice: Use the "question format" when possible. Instead of "This is wrong," ask "Could this cause an issue if X happens?"

The Art of Giving Constructive Feedback

1. Be Kind and Respectful
   ❌ "This code is terrible"
   ✅ "I think we can improve this by..."

2. Focus on Code, Not People
   ❌ "You didn't handle errors"
   ✅ "This function doesn't handle the error case"

3. Explain the "Why"
   ❌ "Don't use var_dump()"
   ✅ "Let's use Log::debug() instead of var_dump() so we don't
       accidentally expose data in production"

4. Offer Solutions, Not Just Problems
   ❌ "This won't scale"
   ✅ "This might cause performance issues with large datasets.
       Consider using pagination or eager loading"

5. Use Positive Language
   ❌ "You forgot to..."
   ✅ "Let's also add..."

6. Praise Good Work
   ✅ "Nice! This abstraction makes the code much cleaner"
   ✅ "Great catch on that edge case"

Remember: The goal is to improve code quality, not to prove you're smart or to criticize the author.

Review Comment Examples

Security Issue:
"⚠️ Security concern: This query is vulnerable to SQL injection.
Let's use parameter binding:

// Instead of:
DB::raw("SELECT * FROM users WHERE id = " . $id)

// Use:
DB::table('users')->where('id', $id)->first()
"

Performance Issue:
"This could cause N+1 queries. Consider eager loading:

// Instead of:
foreach ($posts as $post) {
    echo $post->author->name;
}

// Use:
$posts = Post::with('author')->get();
"

Code Quality:
"This function is doing multiple things. Consider extracting:

public function processOrder($order) {
    $this->validateOrder($order);
    $this->calculateTotals($order);
    $this->sendConfirmation($order);
}

This makes it easier to test and maintain."

Praise:
"💯 Excellent use of the repository pattern here!
This makes testing much easier."

Responding to Review Comments

How to handle feedback professionally:

1. Don't Take It Personally
   - Reviews are about code quality, not your worth
   - Everyone gets feedback, even senior developers
   - View it as learning opportunity

2. Ask for Clarification
   "I'm not sure I understand. Could you elaborate on
   why this approach is better?"

3. Explain Your Reasoning
   "I chose this approach because of X, Y, Z.
   However, I'm open to alternatives if you think
   there's a better way."

4. Be Open to Change
   "That's a great point. I'll update it."
   "I hadn't considered that case. Let me fix it."

5. Push Back Respectfully (When Appropriate)
   "I understand your concern, but in this case,
   we discussed in the team meeting that this
   approach aligns with our architecture goals."

6. Mark Resolved
   - Fix the issue
   - Reply with what you changed
   - Mark the conversation as resolved

Avoid: Getting defensive, arguing unnecessarily, or ignoring feedback. These behaviors damage team dynamics.

GitHub Review States

Comment:
- General feedback without explicit approval
- Use for: Questions, suggestions, discussions

Approve:
- ✅ Code looks good to merge
- Use when: You've reviewed thoroughly and approve

Request Changes:
- ❌ Issues must be fixed before merge
- Use for: Bugs, security issues, breaking problems

Pro Tip:
Submit review as batch rather than one comment at a time
- Review entire PR first
- Click "Start a review"
- Add all comments
- Click "Finish review" and choose state

Review Workflow Example

1. Reviewer Perspective:

# Pull the branch locally
git fetch origin
git checkout feature/new-payment-gateway

# Run tests
php artisan test

# Review changes on GitHub
- Read PR description
- Check files changed
- Add comments (Start a review)
- Test locally if complex
- Submit review (Approve/Request changes)

2. Author Perspective:

# Address feedback
git checkout feature/new-payment-gateway

# Make changes
# ... edit files ...

git add .
git commit -m "Address review feedback: add validation"
git push origin feature/new-payment-gateway

# Reply to comments
- "Fixed in commit abc123"
- Mark conversations as resolved

# Request re-review
- Click "Re-request review" button

Automated Code Review Tools

Automate routine checks so humans focus on logic and design:

Popular Tools:

PHP CodeSniffer: Code style and standards
PHPStan: Static analysis, type checking
Psalm: Static analysis, type safety
PHP CS Fixer: Automatic code formatting
SonarQube: Code quality and security
Codecov: Test coverage reports

GitHub Actions Example:

name: Code Review Automation

on: [pull_request]

jobs:
  quality:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3

      - name: Run PHPStan
        run: ./vendor/bin/phpstan analyse

      - name: Check code style
        run: ./vendor/bin/php-cs-fixer fix --dry-run

      - name: Run tests with coverage
        run: php artisan test --coverage

Best Practice: Automate style and syntax checks. Humans should focus on logic, architecture, and maintainability.

Review Size Best Practices

Ideal PR Size:
- Under 400 lines of code
- Focused on single feature/fix
- Reviewable in 30 minutes or less

If PR is Too Large:
1. Break into smaller PRs
2. Review in stages (architecture first, then details)
3. Schedule pair programming session
4. Use "Draft PR" to get early feedback

PR Description Template:

## What
Brief description of changes

## Why
Context and motivation

## How
Technical approach

## Testing
How to test this change

## Screenshots (if UI)
Before/after images

## Checklist
- [ ] Tests added/updated
- [ ] Documentation updated
- [ ] No breaking changes (or documented)

Code Review Etiquette

For Reviewers:
✓ Review promptly (within 24 hours)
✓ Be thorough but not nitpicky
✓ Distinguish between "must fix" and "nice to have"
✓ Consider saying "LGTM" (Looks Good To Me) when appropriate
✓ End with encouraging words

For Authors:
✓ Keep PRs small and focused
✓ Write clear descriptions
✓ Respond to all comments
✓ Don't force-push after review starts
✓ Say "thank you" for reviews

For Teams:
✓ Define review guidelines
✓ Set response time expectations
✓ Use labels (needs-review, reviewed-approved)
✓ Rotate reviewers for knowledge sharing
✓ Celebrate good reviews in team meetings

Practice Exercise:

Review This Code:

public function store(Request $request) {
    $user = new User;
    $user->name = $request->name;
    $user->email = $request->email;
    $user->password = $request->password;
    $user->save();

    return redirect('/dashboard');
}

Your Task: Write review comments identifying issues and suggesting improvements.

Sample Review Comments:

Security Issue: "⚠️ The password is stored in plain text. We must hash it: $user->password = Hash::make($request->password);"
Validation Missing: "We should validate the input before saving. Consider adding: $request->validate(['name' => 'required', 'email' => 'required|email|unique:users', 'password' => 'required|min:8']);"
Error Handling: "What happens if save() fails? Let's add error handling and a success message."
Modern Approach: "Consider using mass assignment: User::create($request->validated()); (with $fillable property set)"

Summary

In this lesson, you learned:

Code reviews improve quality, catch bugs, and share knowledge
Review for functionality, design, quality, testing, security, and performance
Give constructive feedback: be kind, explain why, offer solutions
Respond professionally: don't take it personally, ask questions, be open
Use GitHub review features: comments, suggestions, approve/request changes
Automate routine checks so humans focus on logic and architecture
Keep PRs small (under 400 lines) for effective reviews

Next Up: In the next lesson, we'll tackle resolving merge conflicts with confidence!