code-review

SKILL.md 节选

# Code Review

Checklists, patterns, feedback techniques, and automation for effective code reviews.

---

## Review Checklist by Category

### Logic and Correctness
- [ ] Does the code do what the PR description says it does?
- [ ] Are edge cases handled (empty inputs, boundary values, concurrent access)?
- [ ] Are error conditions handled gracefully with appropriate error types?
- [ ] Are there off-by-one errors in loops, slices, or pagination?
- [ ] Are null/undefined/nil cases handled?
- [ ] Does the control flow match the intended behavior (early returns, fallthrough)?

### Security
- [ ] No hardcoded secrets, keys, tokens, or passwords
- [ ] Input validation on all user-supplied data (query params, body, headers)
- [ ] No SQL injection, XSS, or command injection vectors
- [ ] Authentication and authorization checks on every endpoint
- [ ] Sensitive data not logged, not in error messages, not exposed in responses
- [ ] CSRF protection on state-changing requests
- [ ] Rate limiting on sensitive endpoints (auth, password reset)

### Performance
- [ ] No N+1 database queries (queries inside loops)
- [ ] No memory leaks (event listeners, timers, subscriptions not cleaned up)
- [ ] Large datasets paginated, not loaded entirely into memory
- [ ] Expensive computations cached where appropriate
- [ ] No blocking operations on the main thread or event loop
- [ ] Database queries use indexes (check EXPLAIN plans for new queries)

### Readability and Maintainability
- [ ] Code is understandable without needing the author to explain it
- [ ] Functions have a single responsibility and a clear name
- [ ] No duplicated logic (DRY where it reduces coupling, not blindly)
- [ ] Naming conventions are consistent with the codebase
- [ ] No dead code, commented-out code, or unused imports
- [ ] Complex logic has comments explaining why, not what

### Testing
- [ ] Tests cover the primary success path
- [ ] Edge cases and error paths tested
- [ ] Tests are deterministic (no flaky tests introduced)
- [ ] Test names describe the behavior, not the implementation
- [ ] Mocks are minimal and realistic

---

## How to Give Constructive Feedback

The praise sandwich is patronizing and everyone sees through it. Be direct and kind. Critique the code, not the person.

### Principles
- Be specific. "This will throw if `user` is null on line 42" beats "handle errors better."
- Explain the why. Don't just say what to change -- explain the risk or benefit.
- Offer alternatives. If you flag a problem, suggest a solution or direction.
- Acknowledge tradeoffs. If a shortcut is reasonable given constraints, say so.
- Ask genuine questions. "What's the reasoning for X?" is valid when you don't understand.

### Prefixed Comments

```
[nit] Minor style issue, take or leave
[suggestion] Optional improvement idea
[question] Seeking understanding, not necessarily a change
[issue] Must be addressed before merge
[praise] Something done well worth calling out
[thought] Sharing context or perspective, no action needed
```

### Direct Feedback Examples

```
Instead of: "This is wrong"
Write: "This will fail when `items` is empty because `items[0]` returns undefined."

Instead of: "Why did you do it this way?"
Write: "Was there a performance reason for the manual loop? Array.filter would be more readable."

Instead of: "This is bad code"
Write: "This function handles validation, transformation, and persistence. Splitting would make each testable independently."
```

---

## PR Size Guidelines

```
Small (< 200 lines changed)   -> Review in 15-30 min. Ideal size.
Medium (200-400 lines)         -> Review in 30-60 min. Acceptable.
Large (400-800 lines)          -> Consider splitting.
Very large (> 800 lines)       -> Should almost always be split.
```

### When to Split

- Refactoring mixed with feature changes. Ship the refactor first.
- Infrastructure changes mixed with business logic.
- Database migrations with application code. Migrate first, then use.
- Multiple independent features or fixes bundled together.

### When Large PRs Are Acceptable

SKILL.md 不止这些内容，安装后可阅读全文。

技能包内容

共 12 个文件，合计 47.0 KiB