程式碼審查技能範例

---
name: Code Review Guidelines
description: Provide thorough, constructive code review feedback
version: 1.0.0
author: Engineering Best Practices
platforms:
  - claude-code
  - codex
categories:
  - development
tags:
  - code-review
  - quality
  - security
---

# Code Review Guidelines

## Review Philosophy

Code review is about improving code quality and sharing knowledge,
not finding faults. Be constructive, specific, and educational.

## Review Checklist

### Security (Critical)
- [ ] Input validation on all user data
- [ ] No SQL injection vulnerabilities
- [ ] No XSS vulnerabilities
- [ ] No hardcoded credentials or secrets
- [ ] Proper authentication/authorization checks
- [ ] Secure random number generation

### Performance
- [ ] No N+1 query problems
- [ ] Appropriate indexing for database queries
- [ ] No unnecessary re-renders (React)
- [ ] Proper memoization where needed
- [ ] Efficient algorithms for data size

### Code Quality
- [ ] Functions do one thing well
- [ ] Meaningful variable/function names
- [ ] No dead code or commented-out code
- [ ] Appropriate error handling
- [ ] Consistent code style

### Testing
- [ ] Tests cover happy path
- [ ] Tests cover edge cases
- [ ] Tests are readable and maintainable
- [ ] No flaky tests

## Feedback Guidelines

### Tone
- Be respectful and constructive
- Explain the "why" behind suggestions
- Ask questions instead of making demands
- Acknowledge good work

### Structure
1. Start with what's good
2. Raise concerns with context
3. Suggest improvements with examples
4. End positively

### Examples

**Good feedback:**
"Consider using a parameterized query here to prevent SQL injection.
The current string concatenation could allow malicious input.

\`\`\`sql
-- Instead of this:
query = "SELECT * FROM users WHERE id = " + userId

-- Consider:
query = "SELECT * FROM users WHERE id = ?"
params = [userId]
\`\`\`"

**Less helpful feedback:**
"This is wrong. Use prepared statements."

## Comment Prefixes

Use these prefixes to indicate severity:

- **[CRITICAL]** - Must fix before merge (security, data loss)
- **[IMPORTANT]** - Should fix, significant issue
- **[SUGGESTION]** - Nice to have improvement
- **[QUESTION]** - Seeking clarification
- **[NIT]** - Minor style preference

## Common Issues

### Security

**SQL Injection:**
\`\`\`javascript
// Vulnerable
db.query(\`SELECT * FROM users WHERE email = '\${email}'\`);

// Safe
db.query('SELECT * FROM users WHERE email = ?', [email]);
\`\`\`

**XSS:**
\`\`\`jsx
// Vulnerable
<div dangerouslySetInnerHTML={{__html: userInput}} />

// Safe
<div>{sanitize(userInput)}</div>
\`\`\`

### Performance

**N+1 Queries:**
\`\`\`javascript
// Problem: N+1 queries
users.forEach(async user => {
  const posts = await db.query('SELECT * FROM posts WHERE user_id = ?', [user.id]);
});

// Solution: Batch query
const posts = await db.query('SELECT * FROM posts WHERE user_id IN (?)', [userIds]);
\`\`\`

### Code Quality

**Magic Numbers:**
\`\`\`javascript
// Unclear
if (status === 3) { ... }

// Clear
const STATUS_COMPLETED = 3;
if (status === STATUS_COMPLETED) { ... }
\`\`\`

## Approval Criteria

Approve when:
- No critical or important issues
- Tests pass
- Code follows style guidelines
- Documentation updated if needed

Request changes when:
- Security vulnerabilities exist
- Critical bugs found
- Tests missing for new functionality
- Breaking changes not documented