Code style
- Code duplication
- Naming of methods
- Naming of variables
- Use of Constants for unmodified variables
- Developer checklist
- Scope and size of the PR (Can it be split?)
- Is it tackling too many features/bugs at once?
- Commit messages: conventional commits
- No commented code checked in
- Async/Awaits handled correctly
- Any video or screenshot to support the bug fix or feature?
Documentation
- API documentation for new/updated endpoints
- PR description
- Ticket reverence (Ex: Jira ticket)
- Updated README.md
- Typos/Grammar
Tests
- Positive cases
- Negative cases
- Error/Exception handling
- Tests naming
- Unit tests
- Service tests if necessary
- Mocking/stubbing
- Increase coverage
Common
- Logical implementation
- Is the code release complaint (Feature flags or environment variables are up to date?)
- No hardcoded values (Date or time)
- Time complexity
- Timezone aware
- Error handling
- Null references
- Default values
- Single responsibility (SOLID principles)
- Open for extension/Closed for modification
- Interface segregation
- Dependency injection (Avoid circular / unused dependencies)
- Dependencies
- Updated package.json => updated lock files (yarn or npm)
- Verify any newly added packages (Is it up to date and licensed)
Bugfixes
- Is the bug fixed in all impacted modules?
- Does it require to communicate to customers?
- Does it impact other teams? Do we need to inform them?
- How well is the bug documented
Security
- Secrets should be in configuration and values should never be in the code.
- No self-made encryption
- No client-side encryption
- SQL Injection
- Authentication
- Authorization
- Sensitive data in response payloads
Ops
- Logging where needed, with appropriate levels
- Environment variables added across all environments
- New packages should not fail the build
Database
- Normalization
- Indexes for queries
- Indexes for new fields
- N+1 queries
- Backward compatibility
- Migration
- Rollback strategy (if needed)
- Data types
- Cascade deletes
API
- RESTful endpoints routes
- HTTP semantics in response codes
- Uniform response format in the service
- Is backward compatibility managed?
Frontend
- Is the output screenshot or video available to verify the changes
- Code / Design should work across all supported devices (Mobile, tablets, etc..)
- APIs should follow the API documentation
- DRY. Is the same code duplicated more than twice?
- Are functions/classes/components reasonably small (not too big)?
- Code has no any linter errors or warnings
- No console.logs
Additional resources:
The Code Review Pyramid - https://www.morling.dev/blog/the-code-review-pyramid/
Top comments (0)