Skip to content

Code Reviews

Jump to bottom
Spinning Idea edited this page May 13, 2022 · 4 revisions

Code Review Checklist

General

  1. Does the code work?
  2. Description of the code status is included (eg alpha/beta/ready to ship to customers).
  3. Any issues known with current code included (eg any gotchas or features not working yet or things that folks should be aware of code wise).
  4. Code is written following the coding standards/guidelines (naming conventions followed, and files structured correctly, separation of concerns followed). - https://github.com/spinningideas/resources/wiki/Coding-Standards
  5. Code is in sync with existing code patterns/technologies.
  6. DRY. Is the same code duplicated more than twice?
  7. Notes on how to test the code?
  8. Are functions/classes/components reasonably small (less than 500 lines)?
  9. Code that is not used is removed (eg Remove unused packages from NPM).

Coding Style

  1. No hardcoded values, use constants values.
  2. Avoid multiple if/else blocks.
  3. No commented out code.
  4. No unnecessary comments: comments that describe the how.
  5. Add necessary comments where needed. Necessary comments are comments that describe the why.

ES6/7

  1. Use ES6 features.
  2. Use fat arrow instead of var that = this. Be consistent in your usage of arrow function.
  3. Use spread/rest operator.
  4. Use default values.
  5. Use const over let (avoid var).
  6. Use import and export.
  7. Use template literals.
  8. Use destructuring assignment for arrays and objects.
  9. Use Promises or Asyns/Await. Rejection is handled.

React JS - Code Review

  1. Do components have defined propTypes?
  2. Are components right sized? (eg less than ~400 lines).
  3. Functional components for components that don't use state.
  4. No api calls in child components, api calls delegated to Pages?
  5. No state updates in loop
  6. No useless constructor
  7. Minimize logic in the render method
  8. Don’t use mixins, prefer HOC and composition

Third-Party Libraries

  1. Avoid use of lodash and moment and other large third party libraries and create functions as needed.
  2. Is any useful library needed and why?

ESLint

  1. Code has no any linter errors or warnings.
  2. No console.logs.

CSS/CSS in JS

  1. Consistent naming conventions are used (BEM, OOCSS, SMACSS, e.t.c.).
  2. CSS selectors are only as specific as they need to be; grouped logically.
  3. Is any 'CSS in JS' library was used?
  4. Use Hex color codes #000 unless using rgba().
  5. Avoid absolute positioning.
  6. Use flexbox.
  7. Avoid !important.
  8. Do not animate width, height, top, left and others. Use transform instead.
  9. Use same units for all project.
  10. Avoid inline styles.

Testing

  1. Tests are readable, maintainable, trustworthy.
  2. Test names (describe, it) are concise, explicit, descriptive.
  3. Avoid logic in your tests.
  4. Don't test multiple concerns in the same test.
  5. Cover the general case and the edge cases.
  6. Test the behavior, not the internal implementation.
  7. Use a mock to simulate/stub complex class structure, methods or async functions.

Git

  1. Commits are small and divided into logical parts.
  2. Commits messages are small and understandable.
  3. Use branches for new features.
  4. Make sure no dist files, editor/IDE files, etc are checked in. There should be a .gitignore for that.

Other

  1. Security.
  2. Usability.

Useful links

Clone this wiki locally