Code Review Best Practices

Code review is a critical part of software development. It catches bugs, improves code quality, shares knowledge, and builds team cohesion.

Why Code Review?

Benefit	Description
Quality	Catch bugs before they reach production
Knowledge sharing	Learn from each other's code
Consistency	Maintain code standards
Mentoring	Help junior developers grow
Documentation	Comments explain the "why"

The Review Process

As a Reviewer

Understand the context: Read the PR description
Check the big picture: Does the approach make sense?
Review the code: Look at the implementation
Test if needed: Pull and run locally
Leave constructive feedback: Be helpful, not harsh
Approve or request changes: Make a decision

As an Author

Self-review first: Check your own code
Write a good description: Help reviewers understand
Keep PRs small: Easier to review
Respond promptly: Keep the process moving
Be open to feedback: Code review is collaborative

GitHub Review Features

Starting a Review

Go to "Files changed" tab
Click the + on any line to add a comment
Choose "Add single comment" or "Start a review"

Review Types

Type	Meaning
Comment	General feedback, no approval decision
Approve	Changes look good, ready to merge
Request changes	Issues must be fixed before merge

Suggesting Changes

Use the suggestion feature:

```suggestion
const result = items.filter(item => item.active);


Authors can apply suggestions with one click.

### Viewing Specific Commits

Review individual commits if the PR has many:
- Commits tab → Click commit
- Useful for large PRs

## What to Look For

### Logic and Correctness

- Does the code do what it's supposed to?
- Are there edge cases not handled?
- Could there be off-by-one errors?
- Are error cases handled?

### Security

- Input validation
- SQL injection risks
- XSS vulnerabilities
- Secrets in code
- Authentication/authorization issues

### Performance

- Unnecessary loops or computations
- N+1 query problems
- Memory leaks
- Large data structure issues

### Maintainability

- Is the code readable?
- Are names clear and meaningful?
- Is there appropriate documentation?
- Is the code too complex?

### Testing

- Are there tests?
- Do tests cover the changes?
- Are edge cases tested?
- Are tests readable?

### Style

- Follows team conventions
- Consistent formatting
- No commented-out code
- No debug statements

## Writing Good Comments

### Be Specific

❌ "This is wrong."
✅ "This will fail when `items` is empty. Consider adding a null check."

### Be Constructive

❌ "This code is terrible."
✅ "This could be simplified by using `reduce()`. Here's an example..."

### Explain Why

❌ "Don't use `var`."
✅ "Use `const` instead of `var` to prevent accidental reassignment and follow our ES6 conventions."

### Use Questions

❌ "This is confusing."
✅ "Could you explain why this check is needed? I want to make sure I understand the logic."

### Acknowledge Good Code

✅ "Great solution! This is much cleaner than the previous approach."
✅ "Nice catch on the edge case handling."

## Comment Prefixes

Use prefixes to indicate importance:

| Prefix | Meaning |
|--------|---------|
| **nit:** | Minor suggestion, not blocking |
| **question:** | Seeking clarification |
| **suggestion:** | Optional improvement |
| **important:** | Must be addressed |
| **blocking:** | Cannot merge until fixed |

Example:

nit: Consider using template literals here for readability.

blocking: This will cause a security vulnerability. We need to sanitize user input before rendering.


## Review Etiquette

### For Reviewers

1. **Be timely**: Review within 24 hours if possible
2. **Be thorough**: Don't just skim
3. **Be kind**: Remember there's a person behind the code
4. **Be clear**: Make actionable suggestions
5. **Assume good intent**: Authors are trying their best

### For Authors

1. **Don't take it personally**: Feedback is about code, not you
2. **Respond to all comments**: Even if just "Done" or "Acknowledged"
3. **Explain your reasoning**: If you disagree, discuss
4. **Thank reviewers**: They're helping you improve

## Handling Disagreements

When you disagree with feedback:

1. **Seek to understand**: Ask clarifying questions
2. **Explain your perspective**: Share your reasoning
3. **Find common ground**: Look for compromise
4. **Escalate if needed**: Get a third opinion
5. **Document decisions**: For future reference

## Large PRs

For PRs that are necessarily large:

### As Author

- Add extra documentation
- Split into logical commits
- Highlight key changes
- Offer to walk through

### As Reviewer

- Review commit-by-commit
- Focus on high-risk areas first
- Take breaks to stay fresh
- Use the "viewed" checkbox

## Automation

Automate what you can:

- **Linting**: Catch style issues automatically
- **Formatting**: Prettier, Black, etc.
- **Testing**: Run tests on every PR
- **Security scanning**: Detect vulnerabilities

This lets reviewers focus on logic and design.

## Summary

- Code review improves quality and shares knowledge
- Use GitHub's review features effectively
- Look for logic, security, performance, maintainability
- Write constructive, specific comments
- Use prefixes to indicate importance
- Be kind and professional
- Automate style and formatting checks

In the next lesson, we'll learn about PR templates.

Benefit

Description

Quality

Catch bugs before they reach production

Knowledge sharing

Learn from each other's code

Consistency

Maintain code standards

Mentoring

Help junior developers grow

Documentation

Comments explain the "why"

The Review Process

As a Reviewer

Understand the context: Read the PR description

Check the big picture: Does the approach make sense?

Review the code: Look at the implementation

Test if needed: Pull and run locally

Leave constructive feedback: Be helpful, not harsh

Approve or request changes: Make a decision

As an Author

Self-review first: Check your own code

Write a good description: Help reviewers understand

Keep PRs small: Easier to review

Respond promptly: Keep the process moving

Be open to feedback: Code review is collaborative

GitHub Review Features

Starting a Review

Go to "Files changed" tab

Click the + on any line to add a comment

Choose "Add single comment" or "Start a review"

Review Types

Type

Meaning

Comment

General feedback, no approval decision

Approve

Changes look good, ready to merge

Request changes

Issues must be fixed before merge

Suggesting Changes

Use the suggestion feature:

```suggestion const result = items.filter(item => item.active);

Authors can apply suggestions with one click. ### Viewing Specific Commits Review individual commits if the PR has many: - Commits tab → Click commit - Useful for large PRs ## What to Look For ### Logic and Correctness - Does the code do what it's supposed to? - Are there edge cases not handled? - Could there be off-by-one errors? - Are error cases handled? ### Security - Input validation - SQL injection risks - XSS vulnerabilities - Secrets in code - Authentication/authorization issues ### Performance - Unnecessary loops or computations - N+1 query problems - Memory leaks - Large data structure issues ### Maintainability - Is the code readable? - Are names clear and meaningful? - Is there appropriate documentation? - Is the code too complex? ### Testing - Are there tests? - Do tests cover the changes? - Are edge cases tested? - Are tests readable? ### Style - Follows team conventions - Consistent formatting - No commented-out code - No debug statements ## Writing Good Comments ### Be Specific ❌ "This is wrong." ✅ "This will fail when `items` is empty. Consider adding a null check." ### Be Constructive ❌ "This code is terrible." ✅ "This could be simplified by using `reduce()`. Here's an example..." ### Explain Why ❌ "Don't use `var`." ✅ "Use `const` instead of `var` to prevent accidental reassignment and follow our ES6 conventions." ### Use Questions ❌ "This is confusing." ✅ "Could you explain why this check is needed? I want to make sure I understand the logic." ### Acknowledge Good Code ✅ "Great solution! This is much cleaner than the previous approach." ✅ "Nice catch on the edge case handling." ## Comment Prefixes Use prefixes to indicate importance: | Prefix | Meaning | |--------|---------| | **nit:** | Minor suggestion, not blocking | | **question:** | Seeking clarification | | **suggestion:** | Optional improvement | | **important:** | Must be addressed | | **blocking:** | Cannot merge until fixed | Example:

nit: Consider using template literals here for readability.

blocking: This will cause a security vulnerability. We need to sanitize user input before rendering.

## Review Etiquette ### For Reviewers 1. **Be timely**: Review within 24 hours if possible 2. **Be thorough**: Don't just skim 3. **Be kind**: Remember there's a person behind the code 4. **Be clear**: Make actionable suggestions 5. **Assume good intent**: Authors are trying their best ### For Authors 1. **Don't take it personally**: Feedback is about code, not you 2. **Respond to all comments**: Even if just "Done" or "Acknowledged" 3. **Explain your reasoning**: If you disagree, discuss 4. **Thank reviewers**: They're helping you improve ## Handling Disagreements When you disagree with feedback: 1. **Seek to understand**: Ask clarifying questions 2. **Explain your perspective**: Share your reasoning 3. **Find common ground**: Look for compromise 4. **Escalate if needed**: Get a third opinion 5. **Document decisions**: For future reference ## Large PRs For PRs that are necessarily large: ### As Author - Add extra documentation - Split into logical commits - Highlight key changes - Offer to walk through ### As Reviewer - Review commit-by-commit - Focus on high-risk areas first - Take breaks to stay fresh - Use the "viewed" checkbox ## Automation Automate what you can: - **Linting**: Catch style issues automatically - **Formatting**: Prettier, Black, etc. - **Testing**: Run tests on every PR - **Security scanning**: Detect vulnerabilities This lets reviewers focus on logic and design. ## Summary - Code review improves quality and shares knowledge - Use GitHub's review features effectively - Look for logic, security, performance, maintainability - Write constructive, specific comments - Use prefixes to indicate importance - Be kind and professional - Automate style and formatting checks In the next lesson, we'll learn about PR templates.