code-review-checklist

Name: code-review-checklist
Author: tranhieutt

69stars41forks

How to install code-review-checklist?

tranhieutt/software_development_department/code-review-checklist

$npx -y skills add tranhieutt/software_development_department --skill code-review-checklist

Installs into the current project.

Summary

Provides a comprehensive code review checklist for pull requests covering security, performance, maintainability, and testing. Use as a reference during code reviews or when the user asks for a review checklist.

Files · 1

View on GitHub

SKILL.md

1	# Code Review Checklist
2
3	## Pre-review (always start here)
4
5	- [ ] Read PR description and linked issue — understand the why
6	- [ ] Check CI passes before spending time on review
7	- [ ] Pull branch locally if logic is complex
8
9	## Functionality
10
11	- [ ] Solves stated problem and meets acceptance criteria
12	- [ ] Edge cases: null/empty inputs, concurrent calls, network failure
13	- [ ] Error handling: errors caught, message doesn't expose internals
14	- [ ] No off-by-one, loop termination, or race conditions
15
16	## Security (block if any fail)
17
18	- [ ] No SQL injection — use parameterized queries, not string concat
19	- [ ] No XSS — escape all user-controlled output in DOM
20	- [ ] No hardcoded secrets — use environment variables
21	- [ ] Authentication required on all protected routes
22	- [ ] Authorization checks presence AND ownership (not just auth)
23	- [ ] File uploads validated: type, size, content
24
25	```javascript
26	// ❌ SQL injection
27	const q = `SELECT * FROM users WHERE email = '${email}'`;
28
29	// ✅ Parameterized
30	db.query("SELECT * FROM users WHERE email = $1", [email]);
31
32	// ❌ Hardcoded secret
33	const KEY = "sk_live_abc123";
34
35	// ✅ Env variable
36	const KEY = process.env.API_KEY;
37	if (!KEY) throw new Error("API_KEY is required");
38	```
39
40	## Performance
41
42	- [ ] No N+1 queries — check ORM calls inside loops
43	- [ ] Database queries use indexes for filter/sort columns
44	- [ ] No unbounded queries — always paginate or limit
45	- [ ] No blocking main thread with sync I/O (Node.js)
46	- [ ] Caching used for repeated expensive operations
47
48	## Code quality
49
50	- [ ] Names describe intent (`calculateTotalPrice` not `calc`)
51	- [ ] Functions have single responsibility (< ~30 lines is a signal)
52	- [ ] No dead code or commented-out blocks
53	- [ ] DRY — no copy-paste of more than 3 lines
54	- [ ] Follows existing project conventions and patterns
55	- [ ] Abstractions are deep enough to justify themselves; thin pass-through wrappers fail the deletion test
56
57	## Tests
58
59	- [ ] New behavior has test coverage
60	- [ ] Happy path + at least 1 failure/edge case tested
61	- [ ] Tests use real assertions, not just "doesn't throw"
62	- [ ] No brittle tests that break on unrelated changes
63
64	## Documentation
65
66	- [ ] Complex logic has `// why` comment (not `// what`)
67	- [ ] Public API changes documented
68	- [ ] Breaking changes documented in CHANGELOG or PR body
69
70	## Review comment format
71
72	```markdown
73	Issue: [What's wrong]
74	Current: `problematic code`
75	Suggested: `improved code`
76	Why: [reason]
77	```
78
79	## Verdict
80
81	- APPROVED — all sections pass
82	- APPROVED WITH CONDITIONS — minor items, non-blocking
83	- CHANGES REQUIRED — blocking security, correctness, or test coverage issues
84
85	Output: checklist score (X/Y passing) + blocking items with file:line refs + verdict

People who liked this also viewed

6 picks

Type

code-review-checklist

69stars41forks

View on GitHub

How to install code-review-checklist?

tranhieutt/software_development_department/code-review-checklist

$npx -y skills add tranhieutt/software_development_department --skill code-review-checklist

Installs into the current project.

Summary

Files · 1

View on GitHub

SKILL.md

1	# Code Review Checklist
2
3	## Pre-review (always start here)
4
5	- [ ] Read PR description and linked issue — understand the why
6	- [ ] Check CI passes before spending time on review
7	- [ ] Pull branch locally if logic is complex
8
9	## Functionality
10
11	- [ ] Solves stated problem and meets acceptance criteria
12	- [ ] Edge cases: null/empty inputs, concurrent calls, network failure
13	- [ ] Error handling: errors caught, message doesn't expose internals
14	- [ ] No off-by-one, loop termination, or race conditions
15
16	## Security (block if any fail)
17
18	- [ ] No SQL injection — use parameterized queries, not string concat
19	- [ ] No XSS — escape all user-controlled output in DOM
20	- [ ] No hardcoded secrets — use environment variables
21	- [ ] Authentication required on all protected routes
22	- [ ] Authorization checks presence AND ownership (not just auth)
23	- [ ] File uploads validated: type, size, content
24
25	```javascript
26	// ❌ SQL injection
27	const q = `SELECT * FROM users WHERE email = '${email}'`;
28
29	// ✅ Parameterized
30	db.query("SELECT * FROM users WHERE email = $1", [email]);
31
32	// ❌ Hardcoded secret
33	const KEY = "sk_live_abc123";
34
35	// ✅ Env variable
36	const KEY = process.env.API_KEY;
37	if (!KEY) throw new Error("API_KEY is required");
38	```
39
40	## Performance
41
42	- [ ] No N+1 queries — check ORM calls inside loops
43	- [ ] Database queries use indexes for filter/sort columns
44	- [ ] No unbounded queries — always paginate or limit
45	- [ ] No blocking main thread with sync I/O (Node.js)
46	- [ ] Caching used for repeated expensive operations
47
48	## Code quality
49
50	- [ ] Names describe intent (`calculateTotalPrice` not `calc`)
51	- [ ] Functions have single responsibility (< ~30 lines is a signal)
52	- [ ] No dead code or commented-out blocks
53	- [ ] DRY — no copy-paste of more than 3 lines
54	- [ ] Follows existing project conventions and patterns
55	- [ ] Abstractions are deep enough to justify themselves; thin pass-through wrappers fail the deletion test
56
57	## Tests
58
59	- [ ] New behavior has test coverage
60	- [ ] Happy path + at least 1 failure/edge case tested
61	- [ ] Tests use real assertions, not just "doesn't throw"
62	- [ ] No brittle tests that break on unrelated changes
63
64	## Documentation
65
66	- [ ] Complex logic has `// why` comment (not `// what`)
67	- [ ] Public API changes documented
68	- [ ] Breaking changes documented in CHANGELOG or PR body
69
70	## Review comment format
71
72	```markdown
73	Issue: [What's wrong]
74	Current: `problematic code`
75	Suggested: `improved code`
76	Why: [reason]
77	```
78
79	## Verdict
80
81	- APPROVED — all sections pass
82	- APPROVED WITH CONDITIONS — minor items, non-blocking
83	- CHANGES REQUIRED — blocking security, correctness, or test coverage issues
84
85	Output: checklist score (X/Y passing) + blocking items with file:line refs + verdict

People who liked this also viewed

6 picks

Type