Skip to content

code-review.md

Latest commit

 

History

History
132 lines (79 loc) · 3.91 KB

code-review.md

File metadata and controls

132 lines (79 loc) · 3.91 KB
Raw

Code Review Guidelines

Naming conventions

  1. Use camelCase for variables, functions, methods, and instances.

  2. Use PascalCase for classes and enums.

  3. Use PascalCase for typescript types and interfaces.

  4. Use PascalCase for zod specifications.

  5. Use PascalCase for React components.

  6. use camelCase for .ts filenames.

  7. use PascalCase for .tsx filenames.

  8. Use <ComponentName>Props for React component props.

  9. Use kebab-case for project root folders.

CI/CD checks

  1. code should be formatted correctly.

  2. code should be type-checked.

  3. code should pass all unit and integration tests.

  4. code should pass all linting checks.

Error Handling

  1. Make sure the absence of value is represented using the Option type.

  2. Application-level code should throw exceptions, with exception handlers placed at strategic locations.

  3. Make sure all recoverable errors are properly handled. The use of the Result type is encouraged and try/catch is discouraged.

  4. Make sure all data from the external world, like user input, network requests, etc are fully validated. All validation must happen at boundaries.

  5. Make sure invariants are in place. For eg: all arguments are validated. Make sure these errors are not usually handled as they are not recoverable.

Testing

  1. Make sure all complex code has accompanying tests.

  2. Make sure all user-facing code has integration tests.

  3. Make sure all developer-facing code has unit tests.

Correctness

  1. Make sure the code is simple and there's no unnecessary complexity.

  2. Make sure data is either immutable or not shared.

  3. All global data should be immutable.

  4. Mutability is handled through pointers.

  5. Make sure the code is as strongly typed as possible. There are no uses of the 'top' type like 'any' or 'object'.

  6. Make sure computations are pure functions. There are no side effects like mutating the global state or making network requests.

  7. Make sure effect functions have no computation/logic.

Generic

  1. Make sure reusable code is extracted into functions and modules.

  2. Make sure reusable code is well-typed, well tested and well-documented. These functions usually should not include invariants. Use the result type to raise errors.

  3. Create reusable best practices patterns in code. It should be simpler to follow best practices than not to follow them.

  4. Make sure existing internal reusable libraries are used instead of duplicating code.

Concurrency

  1. Use a single ownership model or immutable data structures.
  2. Make sure there are no blocking functions.
  3. Make sure there is no shared mutable state.
  4. Make sure there are no locks. Unless it's very low-level code and it's local.
  5. Use simple Tasks where possible for asynchronous tasks(io or compute). Return Futures/Tasks/Promises.
  6. DO NOT create threads. Use Tasks(or thread pools) instead.
  7. Prefer channels or async enumerable if available.
  8. Make sure asynchronous code and parallel code are separated.

Performance

  1. Make sure there are no obvious performance concerns.
  2. Make sure algorithms with the right time complexity are used.
  3. Make sure immutability doesn't cause serious performance concerns.

Metrics

Make sure code quality metrics are no worse. This includes things like

  1. code coverage,
  2. cyclomatic complexity
  3. code duplication

Miscellaneous

  1. Are comments essential? Can the code be simplified instead?

  2. Make sure third-party libraries included are essential. Discourage the use of libraries that are not essential.

  3. If any configuration is needed, make sure it's well documented in README.

Integration testing

  1. Avoid CSS class names and selectors in integration tests.

  2. Test user-visible behavior. Use locators like getByRole, getByText, getByLabel etc.

  3. Use getByTestId when nothing else works.

  4. Test complete user flows. Do not test individual components or pages.