Code Review

Code Review Best Practices

This skill activates when reviewing code for quality, correctness, security, and maintainability.

When to Use This Skill

Activate when:

• Reviewing pull requests
• Conducting code audits
• Providing feedback on code quality
• Identifying security vulnerabilities
• Suggesting refactoring improvements
• Checking adherence to coding standards

Code Review Checklist

1. Correctness and Functionality

Does the code do what it's supposed to do?

• Logic is correct and handles all cases
• Edge cases are considered
• Error handling is appropriate
• No obvious bugs or logical errors
• Assertions and validations are present
• Return values are correct

Questions to ask:

• What happens if this receives null/nil?
• What if the list is empty?
• What if the number is negative/zero?
• Are there off-by-one errors?
• Are comparisons correct (>, >=, :ok # What went wrong?

end

GOOD: Handle explicitly

case risky_operation() do {:ok, result} -> {:ok, result} {:error, reason} -> Logger.error("Operation failed: #{inspect(reason)}") {:error, reason} end

## Review Etiquette

### DO:

- Be respectful and constructive
- Assume good intent
- Ask questions instead of making demands
- Praise good code
- Explain the "why" behind suggestions
- Offer to pair program on complex issues
- Respond promptly to author's replies

### DON'T:

- Be sarcastic or condescending
- Bike-shed on minor style issues
- Block on personal preferences
- Review your own code without another reviewer
- Approve code you don't understand
- Nitpick excessively

## Self-Review Checklist

Before submitting code for review:

- [ ] Code compiles and runs
- [ ] All tests pass
- [ ] Added tests for new functionality
- [ ] No commented-out code
- [ ] No debug print statements
- [ ] Documentation is updated
- [ ] Commit messages are clear
- [ ] No secrets or sensitive data
- [ ] Code follows project style guide
- [ ] Changes are focused (no unrelated changes)

## Key Principles

- **Correctness first**: Code must work correctly
- **Security matters**: Always consider security implications
- **Be specific**: Provide actionable, concrete feedback
- **Be respectful**: Kind, constructive communication
- **Focus on important issues**: Don't bike-shed
- **Explain reasoning**: Help author learn, don't just dictate
- **Approve good code**: Don't let perfect be enemy of good
- **Collaborate**: You're on the same team
- **Check restraint**: Every new symbol earns its place on the ladder — see `/core:restraint`

## References

- `references/no-todos-scan.md` — Scan PR diffs for new `TODO`/`FIXME`/`XXX`/`HACK`/`KLUDGE`/`DEFERRED` markers; treat as BLOCKER, not nit

## Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

- **Author:** [vinnie357](https://github.com/vinnie357)
- **Source:** [vinnie357/claude-skills](https://github.com/vinnie357/claude-skills)
- **License:** MIT

Install and usage instructions live in the source repository linked above.