Code Review Guide
Reviewer Checklist
- Does the code do what the PR description says?
- Are there sufficient tests for the new code?
- Could this break existing functionality?
- Is error handling comprehensive?
- Are there security concerns? (SQL injection, XSS, auth bypass)
- Is the code readable and well-named?
- Is documentation/comments updated?
- Performance: any N+1 queries, missing indices, blocking calls?
- Does it follow the team's coding standards?
- Are dependencies changes justified?
Author Checklist (Before Submitting)
- Self-reviewed the diff
- All tests pass locally
- No debug code or console.logs left
- PR is small and focused (< 400 lines)
- Description explains why, not just what
- Screenshots added for UI changes
- Breaking changes clearly documented
Comment Tone Guide
| Instead of | Try |
|---|---|
| This is wrong | Consider handling the case where... |
| Why did you do this? | Can you help me understand the reason for... |
| This is bad code | This could be simplified to... |
| You must... | I think it would be better to... |
Comment Severity Labels
| Label | Meaning |
|---|---|
| [blocking] | Must be resolved before merge |
| [suggestion] | Optional improvement |
| [nit] | Minor style/naming issue |
| [question] | Asking for clarification |
| [praise] | Good work callout |