DEV Community

Vishesh
Vishesh

Posted on

Code review checklist

Image description

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)