Loading...
Loading...
Comprehensive checklist for conducting thorough code reviews covering functionality, security, performance, and maintainability
npx skill4agent add davila7/claude-code-templates code-review-checklist## Functionality Review
### Requirements
- [ ] Code solves the stated problem
- [ ] All acceptance criteria are met
- [ ] Edge cases are handled
- [ ] Error cases are handled
- [ ] User input is validated
### Logic
- [ ] No logical errors or bugs
- [ ] Conditions are correct (no off-by-one errors)
- [ ] Loops terminate correctly
- [ ] Recursion has proper base cases
- [ ] State management is correct
### Error Handling
- [ ] Errors are caught appropriately
- [ ] Error messages are clear and helpful
- [ ] Errors don't expose sensitive information
- [ ] Failed operations are rolled back
- [ ] Logging is appropriate
### Example Issues to Catch:
**❌ Bad - Missing validation:**
\`\`\`javascript
function createUser(email, password) {
// No validation!
return db.users.create({ email, password });
}
\`\`\`
**✅ Good - Proper validation:**
\`\`\`javascript
function createUser(email, password) {
if (!email || !isValidEmail(email)) {
throw new Error('Invalid email address');
}
if (!password || password.length < 8) {
throw new Error('Password must be at least 8 characters');
}
return db.users.create({ email, password });
}
\`\`\`## Security Review
### Input Validation
- [ ] All user inputs are validated
- [ ] SQL injection is prevented (use parameterized queries)
- [ ] XSS is prevented (escape output)
- [ ] CSRF protection is in place
- [ ] File uploads are validated (type, size, content)
### Authentication & Authorization
- [ ] Authentication is required where needed
- [ ] Authorization checks are present
- [ ] Passwords are hashed (never stored plain text)
- [ ] Sessions are managed securely
- [ ] Tokens expire appropriately
### Data Protection
- [ ] Sensitive data is encrypted
- [ ] API keys are not hardcoded
- [ ] Environment variables are used for secrets
- [ ] Personal data follows privacy regulations
- [ ] Database credentials are secure
### Dependencies
- [ ] No known vulnerable dependencies
- [ ] Dependencies are up to date
- [ ] Unnecessary dependencies are removed
- [ ] Dependency versions are pinned
### Example Issues to Catch:
**❌ Bad - SQL injection risk:**
\`\`\`javascript
const query = \`SELECT * FROM users WHERE email = '\${email}'\`;
db.query(query);
\`\`\`
**✅ Good - Parameterized query:**
\`\`\`javascript
const query = 'SELECT * FROM users WHERE email = $1';
db.query(query, [email]);
\`\`\`
**❌ Bad - Hardcoded secret:**
\`\`\`javascript
const API_KEY = 'sk_live_abc123xyz';
\`\`\`
**✅ Good - Environment variable:**
\`\`\`javascript
const API_KEY = process.env.API_KEY;
if (!API_KEY) {
throw new Error('API_KEY environment variable is required');
}
\`\`\`## Code Quality Review
### Readability
- [ ] Code is easy to understand
- [ ] Variable names are descriptive
- [ ] Function names explain what they do
- [ ] Complex logic has comments
- [ ] Magic numbers are replaced with constants
### Structure
- [ ] Functions are small and focused
- [ ] Code follows DRY principle (Don't Repeat Yourself)
- [ ] Proper separation of concerns
- [ ] Consistent code style
- [ ] No dead code or commented-out code
### Maintainability
- [ ] Code is modular and reusable
- [ ] Dependencies are minimal
- [ ] Changes are backwards compatible
- [ ] Breaking changes are documented
- [ ] Technical debt is noted
### Example Issues to Catch:
**❌ Bad - Unclear naming:**
\`\`\`javascript
function calc(a, b, c) {
return a * b + c;
}
\`\`\`
**✅ Good - Descriptive naming:**
\`\`\`javascript
function calculateTotalPrice(quantity, unitPrice, tax) {
return quantity * unitPrice + tax;
}
\`\`\`
**❌ Bad - Function doing too much:**
\`\`\`javascript
function processOrder(order) {
// Validate order
if (!order.items) throw new Error('No items');
// Calculate total
let total = 0;
for (let item of order.items) {
total += item.price * item.quantity;
}
// Apply discount
if (order.coupon) {
total *= 0.9;
}
// Process payment
const payment = stripe.charge(total);
// Send email
sendEmail(order.email, 'Order confirmed');
// Update inventory
updateInventory(order.items);
return { orderId: order.id, total };
}
\`\`\`
**✅ Good - Separated concerns:**
\`\`\`javascript
function processOrder(order) {
validateOrder(order);
const total = calculateOrderTotal(order);
const payment = processPayment(total);
sendOrderConfirmation(order.email);
updateInventory(order.items);
return { orderId: order.id, total };
}
\`\`\`**Issue:** [Describe the problem]
**Current code:**
\`\`\`javascript
// Show problematic code
\`\`\`
**Suggested fix:**
\`\`\`javascript
// Show improved code
\`\`\`
**Why:** [Explain why this is better]**Question:** [Your question]
**Context:** [Why you're asking]
**Suggestion:** [If you have one]**Nice!** [What you liked]
This is great because [explain why]@requesting-code-review@receiving-code-review@systematic-debugging@test-driven-development