Code Review Checklist

1---
2name: code-review-checklist
3description: "Comprehensive checklist for conducting thorough code reviews covering functionality, security, performance, and maintainability"
4---
5 
6# Code Review Checklist
7 
8## Overview
9 
10Provide a systematic checklist for conducting thorough code reviews. This skill helps reviewers ensure code quality, catch bugs, identify security issues, and maintain consistency across the codebase.
11 
12## When to Use This Skill
13 
14- Use when reviewing pull requests
15- Use when conducting code audits
16- Use when establishing code review standards for a team
17- Use when training new developers on code review practices
18- Use when you want to ensure nothing is missed in reviews
19- Use when creating code review documentation
20 
21## How It Works
22 
23### Step 1: Understand the Context
24 
25Before reviewing code, I'll help you understand:
26- What problem does this code solve?
27- What are the requirements?
28- What files were changed and why?
29- Are there related issues or tickets?
30- What's the testing strategy?
31 
32### Step 2: Review Functionality
33 
34Check if the code works correctly:
35- Does it solve the stated problem?
36- Are edge cases handled?
37- Is error handling appropriate?
38- Are there any logical errors?
39- Does it match the requirements?
40 
41### Step 3: Review Code Quality
42 
43Assess code maintainability:
44- Is the code readable and clear?
45- Are names descriptive?
46- Is it properly structured?
47- Are functions/methods focused?
48- Is there unnecessary complexity?
49 
50### Step 4: Review Security
51 
52Check for security issues:
53- Are inputs validated?
54- Is sensitive data protected?
55- Are there SQL injection risks?
56- Is authentication/authorization correct?
57- Are dependencies secure?
58 
59### Step 5: Review Performance
60 
61Look for performance issues:
62- Are there unnecessary loops?
63- Is database access optimized?
64- Are there memory leaks?
65- Is caching used appropriately?
66- Are there N+1 query problems?
67 
68### Step 6: Review Tests
69 
70Verify test coverage:
71- Are there tests for new code?
72- Do tests cover edge cases?
73- Are tests meaningful?
74- Do all tests pass?
75- Is test coverage adequate?
76 
77## Examples
78 
79### Example 1: Functionality Review Checklist
80 
81```markdown
82## Functionality Review
83 
84### Requirements
85- [ ] Code solves the stated problem
86- [ ] All acceptance criteria are met
87- [ ] Edge cases are handled
88- [ ] Error cases are handled
89- [ ] User input is validated
90 
91### Logic
92- [ ] No logical errors or bugs
93- [ ] Conditions are correct (no off-by-one errors)
94- [ ] Loops terminate correctly
95- [ ] Recursion has proper base cases
96- [ ] State management is correct
97 
98### Error Handling
99- [ ] Errors are caught appropriately
100- [ ] Error messages are clear and helpful
101- [ ] Errors don't expose sensitive information
102- [ ] Failed operations are rolled back
103- [ ] Logging is appropriate
104 
105### Example Issues to Catch:
106 
107**❌ Bad - Missing validation:**
108\`\`\`javascript
109function createUser(email, password) {
110  // No validation!
111  return db.users.create({ email, password });
112}
113\`\`\`
114 
115**✅ Good - Proper validation:**
116\`\`\`javascript
117function createUser(email, password) {
118  if (!email || !isValidEmail(email)) {
119    throw new Error('Invalid email address');
120  }
121  if (!password || password.length < 8) {
122    throw new Error('Password must be at least 8 characters');
123  }
124  return db.users.create({ email, password });
125}
126\`\`\`
127```
128 
129### Example 2: Security Review Checklist
130 
131```markdown
132## Security Review
133 
134### Input Validation
135- [ ] All user inputs are validated
136- [ ] SQL injection is prevented (use parameterized queries)
137- [ ] XSS is prevented (escape output)
138- [ ] CSRF protection is in place
139- [ ] File uploads are validated (type, size, content)
140 
141### Authentication & Authorization
142- [ ] Authentication is required where needed
143- [ ] Authorization checks are present
144- [ ] Passwords are hashed (never stored plain text)
145- [ ] Sessions are managed securely
146- [ ] Tokens expire appropriately
147 
148### Data Protection
149- [ ] Sensitive data is encrypted
150- [ ] API keys are not hardcoded
151- [ ] Environment variables are used for secrets
152- [ ] Personal data follows privacy regulations
153- [ ] Database credentials are secure
154 
155### Dependencies
156- [ ] No known vulnerable dependencies
157- [ ] Dependencies are up to date
158- [ ] Unnecessary dependencies are removed
159- [ ] Dependency versions are pinned
160 
161### Example Issues to Catch:
162 
163**❌ Bad - SQL injection risk:**
164\`\`\`javascript
165const query = \`SELECT * FROM users WHERE email = '\${email}'\`;
166db.query(query);
167\`\`\`
168 
169**✅ Good - Parameterized query:**
170\`\`\`javascript
171const query = 'SELECT * FROM users WHERE email = $1';
172db.query(query, [email]);
173\`\`\`
174 
175**❌ Bad - Hardcoded secret:**
176\`\`\`javascript
177const API_KEY = 'sk_live_abc123xyz';
178\`\`\`
179 
180**✅ Good - Environment variable:**
181\`\`\`javascript
182const API_KEY = process.env.API_KEY;
183if (!API_KEY) {
184  throw new Error('API_KEY environment variable is required');
185}
186\`\`\`
187```
188 
189### Example 3: Code Quality Review Checklist
190 
191```markdown
192## Code Quality Review
193 
194### Readability
195- [ ] Code is easy to understand
196- [ ] Variable names are descriptive
197- [ ] Function names explain what they do
198- [ ] Complex logic has comments
199- [ ] Magic numbers are replaced with constants
200 
201### Structure
202- [ ] Functions are small and focused
203- [ ] Code follows DRY principle (Don't Repeat Yourself)
204- [ ] Proper separation of concerns
205- [ ] Consistent code style
206- [ ] No dead code or commented-out code
207 
208### Maintainability
209- [ ] Code is modular and reusable
210- [ ] Dependencies are minimal
211- [ ] Changes are backwards compatible
212- [ ] Breaking changes are documented
213- [ ] Technical debt is noted
214 
215### Example Issues to Catch:
216 
217**❌ Bad - Unclear naming:**
218\`\`\`javascript
219function calc(a, b, c) {
220  return a * b + c;
221}
222\`\`\`
223 
224**✅ Good - Descriptive naming:**
225\`\`\`javascript
226function calculateTotalPrice(quantity, unitPrice, tax) {
227  return quantity * unitPrice + tax;
228}
229\`\`\`
230 
231**❌ Bad - Function doing too much:**
232\`\`\`javascript
233function processOrder(order) {
234  // Validate order
235  if (!order.items) throw new Error('No items');
236  
237  // Calculate total
238  let total = 0;
239  for (let item of order.items) {
240    total += item.price * item.quantity;
241  }
242  
243  // Apply discount
244  if (order.coupon) {
245    total *= 0.9;
246  }
247  
248  // Process payment
249  const payment = stripe.charge(total);
250  
251  // Send email
252  sendEmail(order.email, 'Order confirmed');
253  
254  // Update inventory
255  updateInventory(order.items);
256  
257  return { orderId: order.id, total };
258}
259\`\`\`
260 
261**✅ Good - Separated concerns:**
262\`\`\`javascript
263function processOrder(order) {
264  validateOrder(order);
265  const total = calculateOrderTotal(order);
266  const payment = processPayment(total);
267  sendOrderConfirmation(order.email);
268  updateInventory(order.items);
269  
270  return { orderId: order.id, total };
271}
272\`\`\`
273```
274 
275## Best Practices
276 
277### ✅ Do This
278 
279- **Review Small Changes** - Smaller PRs are easier to review thoroughly
280- **Check Tests First** - Verify tests pass and cover new code
281- **Run the Code** - Test it locally when possible
282- **Ask Questions** - Don't assume, ask for clarification
283- **Be Constructive** - Suggest improvements, don't just criticize
284- **Focus on Important Issues** - Don't nitpick minor style issues
285- **Use Automated Tools** - Linters, formatters, security scanners
286- **Review Documentation** - Check if docs are updated
287- **Consider Performance** - Think about scale and efficiency
288- **Check for Regressions** - Ensure existing functionality still works
289 
290### ❌ Don't Do This
291 
292- **Don't Approve Without Reading** - Actually review the code
293- **Don't Be Vague** - Provide specific feedback with examples
294- **Don't Ignore Security** - Security issues are critical
295- **Don't Skip Tests** - Untested code will cause problems
296- **Don't Be Rude** - Be respectful and professional
297- **Don't Rubber Stamp** - Every review should add value
298- **Don't Review When Tired** - You'll miss important issues
299- **Don't Forget Context** - Understand the bigger picture
300 
301## Complete Review Checklist
302 
303### Pre-Review
304- [ ] Read the PR description and linked issues
305- [ ] Understand what problem is being solved
306- [ ] Check if tests pass in CI/CD
307- [ ] Pull the branch and run it locally
308 
309### Functionality
310- [ ] Code solves the stated problem
311- [ ] Edge cases are handled
312- [ ] Error handling is appropriate
313- [ ] User input is validated
314- [ ] No logical errors
315 
316### Security
317- [ ] No SQL injection vulnerabilities
318- [ ] No XSS vulnerabilities
319- [ ] Authentication/authorization is correct
320- [ ] Sensitive data is protected
321- [ ] No hardcoded secrets
322 
323### Performance
324- [ ] No unnecessary database queries
325- [ ] No N+1 query problems
326- [ ] Efficient algorithms used
327- [ ] No memory leaks
328- [ ] Caching used appropriately
329 
330### Code Quality
331- [ ] Code is readable and clear
332- [ ] Names are descriptive
333- [ ] Functions are focused and small
334- [ ] No code duplication
335- [ ] Follows project conventions
336 
337### Tests
338- [ ] New code has tests
339- [ ] Tests cover edge cases
340- [ ] Tests are meaningful
341- [ ] All tests pass
342- [ ] Test coverage is adequate
343 
344### Documentation
345- [ ] Code comments explain why, not what
346- [ ] API documentation is updated
347- [ ] README is updated if needed
348- [ ] Breaking changes are documented
349- [ ] Migration guide provided if needed
350 
351### Git
352- [ ] Commit messages are clear
353- [ ] No merge conflicts
354- [ ] Branch is up to date with main
355- [ ] No unnecessary files committed
356- [ ] .gitignore is properly configured
357 
358## Common Pitfalls
359 
360### Problem: Missing Edge Cases
361**Symptoms:** Code works for happy path but fails on edge cases
362**Solution:** Ask "What if...?" questions
363- What if the input is null?
364- What if the array is empty?
365- What if the user is not authenticated?
366- What if the network request fails?
367 
368### Problem: Security Vulnerabilities
369**Symptoms:** Code exposes security risks
370**Solution:** Use security checklist
371- Run security scanners (npm audit, Snyk)
372- Check OWASP Top 10
373- Validate all inputs
374- Use parameterized queries
375- Never trust user input
376 
377### Problem: Poor Test Coverage
378**Symptoms:** New code has no tests or inadequate tests
379**Solution:** Require tests for all new code
380- Unit tests for functions
381- Integration tests for features
382- Edge case tests
383- Error case tests
384 
385### Problem: Unclear Code
386**Symptoms:** Reviewer can't understand what code does
387**Solution:** Request improvements
388- Better variable names
389- Explanatory comments
390- Smaller functions
391- Clear structure
392 
393## Review Comment Templates
394 
395### Requesting Changes
396```markdown
397**Issue:** [Describe the problem]
398 
399**Current code:**
400\`\`\`javascript
401// Show problematic code
402\`\`\`
403 
404**Suggested fix:**
405\`\`\`javascript
406// Show improved code
407\`\`\`
408 
409**Why:** [Explain why this is better]
410```
411 
412### Asking Questions
413```markdown
414**Question:** [Your question]
415 
416**Context:** [Why you're asking]
417 
418**Suggestion:** [If you have one]
419```
420 
421### Praising Good Code
422```markdown
423**Nice!** [What you liked]
424 
425This is great because [explain why]
426```
427 
428## Related Skills
429 
430- `@requesting-code-review` - Prepare code for review
431- `@receiving-code-review` - Handle review feedback
432- `@systematic-debugging` - Debug issues found in review
433- `@test-driven-development` - Ensure code has tests
434 
435## Additional Resources
436 
437- [Google Code Review Guidelines](https://google.github.io/eng-practices/review/)
438- [OWASP Top 10](https://owasp.org/www-project-top-ten/)
439- [Code Review Best Practices](https://github.com/thoughtbot/guides/tree/main/code-review)
440- [How to Review Code](https://www.kevinlondon.com/2015/05/05/code-review-best-practices.html)
441 
442---
443 
444**Pro Tip:** Use a checklist template for every review to ensure consistency and thoroughness. Customize it for your team's specific needs!
445