Code Review Skill

Overview

This skill provides a systematic approach to code review, ensuring thorough quality assessment while maintaining constructive feedback practices.

Review Checklist

Code Quality

• • Code is readable and self-documenting
• • Functions are small and focused (< 50 lines)
• • Variable names are descriptive
• • No unnecessary complexity
• • DRY principle followed
• • Consistent formatting

Logic & Correctness

• • Logic is sound and correct
• • Edge cases handled
• • Boundary conditions checked
• • No off-by-one errors
• • Return values are correct
• • State mutations are intentional

Error Handling

• • All error cases handled
• • Error messages are helpful
• • No silent failures
• • Async errors are caught
• • Errors don't leak sensitive info

Security (OWASP Top 10)

• • No hardcoded credentials
• • Input is validated/sanitized
• • No injection vulnerabilities (SQL, XSS, command)
• • Authentication/authorization proper
• • Sensitive data protected

Performance

• • No obvious inefficiencies
• • Appropriate data structures
• • No N+1 query patterns
• • Unnecessary operations avoided
• • Memory leaks prevented

Testing

• • Tests exist for new code
• • Tests cover happy path
• • Tests cover edge cases
• • Tests cover error cases
• • Tests are maintainable

Documentation

• • Complex logic explained
• • Public APIs documented
• • No outdated comments
• • README updated if needed

Review Process

Step 1: Understand Context

1. •Read the PR description
2. •Understand the purpose
3. •Check related issues/tickets
4. •Review the overall approach

Step 2: High-Level Review

1. •Does the approach make sense?
2. •Are there architectural concerns?
3. •Is it the right solution?
4. •Are there simpler alternatives?

Step 3: Detailed Review

1. •Go through each file
2. •Check against checklist
3. •Note issues with severity
4. •Provide specific feedback

Step 4: Summarize

1. •Categorize findings
2. •Prioritize issues
3. •Acknowledge good work
4. •Provide clear recommendations

Severity Levels

Feedback Guidelines

Good Feedback

# Constructive
"This function is getting complex. Consider extracting the validation
logic into a separate `validateInput()` function for better readability."

# Specific with solution
"Line 45: This SQL query is vulnerable to injection. Consider using
parameterized queries:
`const result = await db.query('SELECT * FROM users WHERE id = ?', [userId]);`"

# Educational
"Nice use of early returns here! For consistency, consider applying
the same pattern in the `processOrder` function above."

Bad Feedback

# Vague
"This could be better."

# No solution
"This is wrong."

# Harsh
"This is terrible code. Did you even test this?"

# Nitpicking
"Use single quotes instead of double quotes."
(When both are acceptable in the project)

Review Report Template

# Code Review Report

**PR/Commit**: [Reference]
**Reviewer**: [Name]
**Date**: [Date]

## Summary

| Severity | Count |
|----------|-------|
| Blocker | X |
| Critical | X |
| Major | X |
| Minor | X |

**Verdict**: Approve / Request Changes / Needs Discussion

## Findings

### Blockers
[None / List items]

### Critical
[None / List items]

### Major
[None / List items]

### Minor
[None / List items]

## What's Good
- [Positive observation 1]
- [Positive observation 2]

## Recommendations
1. [Action item]
2. [Action item]

Common Issues to Catch

Security

• •Hardcoded secrets
• •SQL injection
• •XSS vulnerabilities
• •Missing authentication
• •Insecure direct object references

Performance

• •N+1 database queries
• •Missing indexes
• •Large synchronous operations
• •Memory leaks
• •Unnecessary API calls

Reliability

• •Missing error handling
• •Race conditions
• •Undefined behavior on edge cases
• •No input validation
• •Silent failures

Maintainability

• •Code duplication
• •God functions (too long)
• •Unclear naming
• •Missing documentation
• •Complex conditionals

Self-Review Before PR

Authors should check:

1. • Tests pass locally
2. • Code is formatted
3. • No debug code left
4. • Self-reviewed the diff
5. • PR description is clear
6. • Related issues linked