Initial commit

skills/code-review.md

@@ -0,0 +1,220 @@
+# Code Review Skill
+
+Perform comprehensive code reviews following industry best practices.
+
+## Review Checklist
+
+### 1. Code Quality
+- [ ] Follows coding standards and style guide
+- [ ] No code duplication (DRY principle)
+- [ ] Functions are small and focused (SRP)
+- [ ] Meaningful variable and function names
+- [ ] Appropriate use of comments (why, not what)
+- [ ] No magic numbers or strings
+- [ ] Proper error handling
+
+### 2. Architecture & Design
+- [ ] SOLID principles applied
+- [ ] Appropriate design patterns used
+- [ ] Separation of concerns
+- [ ] Dependency injection where appropriate
+- [ ] No circular dependencies
+- [ ] Interfaces used for abstraction
+- [ ] Proper layering/modularity
+
+### 3. Security
+- [ ] No hardcoded secrets or credentials
+- [ ] Input validation and sanitization
+- [ ] SQL injection prevention
+- [ ] XSS prevention
+- [ ] CSRF protection where needed
+- [ ] Proper authentication/authorization
+- [ ] Secure data transmission
+- [ ] No sensitive data in logs
+
+### 4. Performance
+- [ ] Efficient algorithms and data structures
+- [ ] No N+1 queries
+- [ ] Proper indexing for database queries
+- [ ] Caching implemented where appropriate
+- [ ] No unnecessary computations
+- [ ] Lazy loading where appropriate
+- [ ] Resource cleanup (connections, files)
+
+### 5. Testing
+- [ ] Unit tests present and passing
+- [ ] Integration tests where needed
+- [ ] Test coverage is adequate (>80% for critical paths)
+- [ ] Edge cases tested
+- [ ] Error conditions tested
+- [ ] Tests are maintainable
+- [ ] No flaky tests
+
+### 6. Error Handling
+- [ ] All errors properly caught and handled
+- [ ] Meaningful error messages
+- [ ] Proper logging at appropriate levels
+- [ ] No swallowed exceptions
+- [ ] Graceful degradation
+- [ ] Retry logic where appropriate
+
+### 7. Documentation
+- [ ] README updated if needed
+- [ ] API documentation present
+- [ ] Complex logic explained
+- [ ] Public APIs documented
+- [ ] Breaking changes noted
+- [ ] Migration guide if needed
+
+### 8. Dependencies
+- [ ] No unnecessary dependencies
+- [ ] Dependencies are up to date
+- [ ] No known vulnerabilities
+- [ ] License compatibility checked
+- [ ] Bundle size impact considered
+
+### 9. Database
+- [ ] Migrations are reversible
+- [ ] Indexes on foreign keys
+- [ ] No missing indexes for queries
+- [ ] Proper transaction handling
+- [ ] Connection pooling used
+- [ ] Data validation at DB level
+
+### 10. API Design
+- [ ] RESTful principles followed
+- [ ] Consistent naming conventions
+- [ ] Proper HTTP methods and status codes
+- [ ] Versioning strategy in place
+- [ ] Rate limiting considered
+- [ ] Pagination for list endpoints
+
+## Review Process
+
+1. **Understand the Context**
+   - Read the PR/commit description
+   - Understand what problem is being solved
+   - Review linked issues/tickets
+
+2. **High-Level Review**
+   - Check overall architecture
+   - Verify design decisions
+   - Look for major issues
+
+3. **Detailed Review**
+   - Go through code line by line
+   - Check against checklist above
+   - Note issues by severity
+
+4. **Security Review**
+   - Look for security vulnerabilities
+   - Check authentication/authorization
+   - Verify input validation
+
+5. **Performance Review**
+   - Look for performance issues
+   - Check database queries
+   - Review caching strategy
+
+6. **Test Review**
+   - Verify test coverage
+   - Check test quality
+   - Run tests locally
+
+7. **Provide Feedback**
+   - Categorize by severity: Critical, Major, Minor, Suggestion
+   - Be specific and constructive
+   - Provide examples or alternatives
+   - Acknowledge good practices
+
+## Severity Levels
+
+**Critical**: Must be fixed before merge
+- Security vulnerabilities
+- Data loss scenarios
+- Breaking changes without migration
+- Production-breaking bugs
+
+**Major**: Should be fixed before merge
+- Performance issues
+- Architectural violations
+- Missing error handling
+- Inadequate test coverage
+
+**Minor**: Can be fixed later
+- Code style issues
+- Missing comments
+- Minor refactoring opportunities
+
+**Suggestion**: Optional improvements
+- Alternative approaches
+- Future enhancements
+- Code organization ideas
+
+## Example Review Comments
+
+### Good Comments
+✅ "Consider using a Map instead of nested loops here for O(1) lookups. Current implementation is O(n²) which could be slow with large datasets."
+
+✅ "This endpoint is missing authentication. Should we add the @RequireAuth decorator?"
+
+✅ "Great use of the strategy pattern here! This makes adding new payment methods much easier."
+
+### Bad Comments
+❌ "This is wrong."
+❌ "Why did you do it this way?"
+❌ "I would have done X instead."
+
+## Automated Checks
+
+Run these before manual review:
+```bash
+# Linting
+npm run lint
+
+# Type checking
+npm run type-check
+
+# Tests
+npm run test
+
+# Security scan
+npm audit
+
+# Code coverage
+npm run test:coverage
+```
+
+## Focus Areas by Language
+
+### JavaScript/TypeScript
+- Proper use of async/await
+- No console.log in production
+- TypeScript types used correctly
+- React hooks rules followed
+
+### Python
+- PEP 8 compliance
+- Type hints used
+- Virtual environment used
+- Requirements.txt updated
+
+### Go
+- Error handling (don't ignore errors)
+- Proper defer usage
+- Channel and goroutine safety
+- Context usage for cancellation
+
+### Rust
+- Ownership rules followed
+- No unsafe blocks without justification
+- Error handling with Result/Option
+- Lifetime annotations correct
+
+## Post-Review
+
+After review is complete:
+- Mark as approved if all critical/major issues resolved
+- Request changes if issues remain
+- Follow up on suggested improvements
+- Document any technical debt created