Skip to content

Instantly share code, notes, and snippets.

@brycefisher
Forked from blakewest/angular_code_review.md
Last active August 29, 2015 14:13
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save brycefisher/643a435dea572c0e9198 to your computer and use it in GitHub Desktop.
Save brycefisher/643a435dea572c0e9198 to your computer and use it in GitHub Desktop.

General

  • Are all vars used somewhere?
  • Do the vars have properly cased, and meaningful names?
  • Style looks appropriate
  • No unnecessarily duplicated logic
  • Code intent is clear upon initial reading
  • Any code that isn't clear, but is needed, has an effective comment
  • Are method calls or attribute lookups every called on entities that could be undefined/null?
  • The functions can appropriately handle unexpected inputs.
  • Commented code has been removed (comments themselves are fine).
  • There are tests for the proposed functionality (if not, there's a good reason)
  • You've actually read the tests, and have a sense of what's being tested and what isn't. (or this is n/a)

Angular Specific

  • No DOM manipulation is happening inside controllers
  • View logic is non-existant or minimal
  • Among the rendering logic, there are no good candidates to be extracted into a filter
  • Among the controller logic, there are no good candidates to be extracted into a service
  • Among all the logic, there are no good candidates to be extracted into a directive
  • Among the partials, there are no good candidates for extraction into an ng-include
  • No unneeded controllers were created.

Final Checks

  • It has indeed passed build tests before getting merged
  • It does what the description says it does
  • Any technical debt has been added to the log.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment