skill-code-review

# skill-code-review


# Code Review Skill Instructions

## Purpose
Conduct comprehensive code reviews that improve code quality, catch bugs early, ensure security, and promote team learning through constructive feedback.

## When to Use
- Pull request reviews
- Pre-merge code inspections
- Security audits
- Refactoring validation
- New feature implementation reviews

---

## Review Process

### 1. Understand Context First
- Read the PR/change description and linked issues
- Understand the **intent** and **scope** of the change
- Check if tests and documentation are included
- Review the overall architecture impact

### 2. Review Checklist

#### ✅ Correctness
- [ ] Does the code do what it's supposed to do?
- [ ] Are edge cases handled properly?
- [ ] Is the logic correct and complete?
- [ ] Are there any off-by-one errors, null pointer issues, or race conditions?

#### 🔒 Security
- [ ] Input validation and sanitization
- [ ] No hardcoded secrets, API keys, or credentials
- [ ] Proper authentication and authorization checks
- [ ] SQL injection, XSS, CSRF protection
- [ ] Secure handling of sensitive data
- [ ] Dependencies free of known vulnerabilities

#### 🏗️ Architecture & Design
- [ ] Follows SOLID principles
- [ ] Appropriate separation of concerns
- [ ] No unnecessary coupling between components
- [ ] Consistent with existing patterns in the codebase
- [ ] Scalability considerations addressed

#### 📖 Readability & Maintainability
- [ ] Clear, descriptive naming (variables, functions, classes)
- [ ] Functions are small and do one thing well
- [ ] No magic numbers or strings (use constants)
- [ ] Complex logic is commented or self-documenting
- [ ] No dead code or commented-out code blocks

#### ⚡ Performance
- [ ] No N+1 queries or unnecessary database calls
- [ ] Efficient algorithms and data structures
- [ ] Proper caching where appropriate
- [ ] No memory leaks or resource exhaustion risks
- [ ] Async operations used correctly

#### 🧪 Testing
- [ ] Unit tests cover new functionality
- [ ] Edge cases and error paths tested
- [ ] Tests are readable and maintainable
- [ ] No flaky or brittle tests
- [ ] Integration tests where appropriate

#### 📝 Documentation
- [ ] Public APIs documented
- [ ] Complex business logic explained
- [ ] README updated if needed
- [ ] Breaking changes documented

---

## Feedback Guidelines

### Be Constructive
```
❌ "This code is bad"
✅ "Consider extracting this into a separate function for better testability"
```

### Be Specific
```
❌ "Fix the naming"
✅ "Rename `data` to `userProfile` to clarify its purpose"
```

### Explain the Why
```
❌ "Don't use var"
✅ "Use `const` instead of `var` to prevent accidental reassignment and improve code clarity"
```

### Categorize Feedback Severity

| Prefix | Meaning | Action Required |
|--------|---------|-----------------|
| 🚨 **BLOCKER** | Critical issue, must fix | Cannot merge |
| ⚠️ **WARNING** | Should fix, potential problem | Strongly recommended |
| 💡 **SUGGESTION** | Improvement opportunity | Optional |
| ❓ **QUESTION** | Clarification needed | Please explain |
| 👍 **PRAISE** | Great work! | Keep it up |

---

## Comment Templates

### Security Issue
```
🚨 **BLOCKER - Security**: User input is not sanitized before being used in the SQL query. 
This creates a SQL injection vulnerability.

**Suggestion**: Use parameterized queries or an ORM to safely handle user input.
```

### Performance Concern
```
⚠️ **WARNING - Performance**: This loop makes a database call on each iteration, 
resulting in N+1 queries.

**Suggestion**: Batch the queries or use eager loading to fetch all data upfront.
```

### Code Quality Suggestion
```
💡 **SUGGESTION**: This function is 80 lines long with multiple responsibilities.

Consider splitting into:
- `validateInput()` - Input validation
- `processData()` - Core business logic  
- `formatResponse()` - Response formatting
```

### Positive Feedback
```
👍 **PRAISE**: Excellent error handling here! The fallback mechanism and 
detailed logging will make debugging much easier.
```

---

## Review Output Format

Structure your review as follows:

```markdown
## Code Review Summary

**Overall Assessment**: ✅ Approved | ⚠️ Needs Changes | 🚨 Request Changes

### Overview
Brief summary of what was reviewed and overall impressions.

### Critical Issues (Must Fix)
- Issue 1 with location and fix suggestion
- Issue 2 with location and fix suggestion

### Recommendations (Should Fix)
- Recommendation 1
- Recommendation 2

### Suggestions (Nice to Have)
- Suggestion 1
- Suggestion 2

### Positive Highlights
- What was done well

### Questions
- Any clarifications needed
```

---

## Best Practices

1. **Review in small batches** - Keep PRs small (<400 lines) for effective review
2. **Take your time** - Don't rush; bugs missed in review are expensive later
3. **Be respectful** - Review the code, not the person
4. **Assume good intent** - Authors did their best with available information
5. **Offer alternatives** - Don't just criticize; provide solutions
6. **Learn together** - Reviews are learning opportunities for everyone
7. **Follow up** - Verify fixes address the concerns raised

---

## Anti-Patterns to Avoid

- ❌ Nitpicking style issues (use linters instead)
- ❌ Rewriting someone's code in your style
- ❌ Blocking PRs for subjective preferences
- ❌ Reviewing without understanding context
- ❌ Being vague or unconstructive
- ❌ Ignoring positive aspects of the code