Skip to content

Instantly share code, notes, and snippets.

@seahyc

seahyc/code_review.md

Created October 27, 2021 01:37
Show Gist options
  • Save seahyc/7a78312e75cec07f7b1eaed91f2f3571 to your computer and use it in GitHub Desktop.
Save seahyc/7a78312e75cec07f7b1eaed91f2f3571 to your computer and use it in GitHub Desktop.
Download ZIP
first draft
Raw
code_review.md

Fulfilment of Specs and Implementation Rigor

  • Does the app fulfill the given requirements?

Complexity/Code Quality

  • Is it easy to understand how the code works?
  • Are things organized in a sensible, consistent manner?
  • Do the files have an appropriate length?
  • Are there no unnecessary DRY violations?
  • Is the code not too tightly coupled? Are concerns separated? Would it be easy to modify the code?
  • Do code-units have an appropriate length?
  • Is there no code that should have been encapsulated in a separate code-unit?
  • Is code-nesting reasonably shallow?
  • Are there no too-convoluted if-else/ternary statements?
  • Are there no "magic strings" or values that shouldn't have been hard-coded?

Tests (if required)

  • Do the tests cover everything they should cover (incl. edge-cases)?
  • Will the tests fail properly if the code is broken?
  • Are the tests well-written and maintainable?
  • Are the test-cases well-named?
  • Are no test-cases too big or test too many concerns?
  • Do the tests test behavior rather than internal implementation?

Naming

  • Are there no misleading names? Do no names break the expectations they raise?
  • Do the names have an appropriate length?
  • Are there no spelling- or grammar-mistakes in any names?
  • Do the names use casing in a sensible and consistent manner?
  • Is the naming consistent (e.g. all booleans are statements like isLoading)?
  • Are the names context-aware (e.g. the isLoading from a useQuery should be renamed to companiesAreLoading if the variable is used far away from it's declaration)?

Comments

  • Are all code-comments absolutely necessary?
  • Do the comments explain the why rather than the how/what?
  • Is there no commented-out code left over?
  • Are all TODO comments appropriate?

Code Style

  • Is the code-style consistent?
  • Does the code-style demonstrate competency in the language?

API knowledge

  • Does the code demonstrate competency at the relevant APIs?
  • Is there no code that could have been improved by using a (different) API?
  • Does the project have no unmaintained or obscure dependencies?
  • Is the code using no outdated/deprecated/unsupported APIs?

Performance

  • Do any requests fetch more than they need to (e.g. data that's not being used or images that are much larger than displayed)?
  • Are there no multiple requests that should have been combined into a single request?
  • Are there no significant Layout Shifts?
  • Are there no requests that block the user inappropriately?
  • Do the FCP and LCP happen within a reasonable time?
  • Are performance problems caused by many or oversized user-inputs avoided/mitigated?

Repo Hygiene

  • Is the readme helpful?
  • Is there nothing committed that shouldn't have been (e.g. passwords/tokens, editor files, build files)?
  • Do the commit message-subjects explain what the change is doing in an understandable way?
  • Are there commit message-bodies added where necessary to explain why a change was made?
  • Do commit-messages follow a consistent style?
  • Are there no commits that are too large (should have been split) or too small (should have been combined)?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment