Skip to content

Instantly share code, notes, and snippets.

@cartermp

cartermp/CodeReviewChecklist.md

Last active October 19, 2018 15:11
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save cartermp/2d29816d85800e6ea4c0 to your computer and use it in GitHub Desktop.
Save cartermp/2d29816d85800e6ea4c0 to your computer and use it in GitHub Desktop.
Download ZIP
CASS Code Review Checklist
Raw
CodeReviewChecklist.md

General Non-Code

  • Does the application compile?
  • Does the application, with changes, run without crashing?
  • Can a database, if applicable, be deployed without error?
  • If a new feature is under review, does the new feature act as expected (if this is easy to test)?
  • Does the commit message indicate the scope of the change set?
  • Does the commit message call out any future work that remains to be done?
  • Does the scope of the change set appear to fulfill the given task, and only that task?
  • Is the change set associated with its appropriate work item?
  • Is the task appropriate to the branch the change set is sitting on?
  • Was one of Get Latest Version, git pull --all , git fetch --all, etc run before the changeset was worked on?
  • Is there an suspicion of unmerged, conflicting code?

General Code

  • Is the code readable and easy to understand?
  • Is there any redundant or duplicate code?
  • Is the code reasonably modular?
  • Is the code following project coding conventions?
  • Are there any replaceable globals?
  • Is there any commented-out code? Approve only with permission from supervisor and an appropriate TODO.
  • Is there any code which could easily be replaced with a well-tested library?
  • Is there any removable logging or debugging code?
  • Can you, the reviewer, think of a better way to accomplish the same goal? How and why?

Databases

  • Is business logic embedded in database code? Is this reasonable?
  • Is there a Trigger added? Why? Can it be removed? If not, is there a good reason why?

Security

  • Are inputs checked where appropriate? (i.e. form validation)
  • Are failure cases, where applicable, being handled?
  • Are function inputs checked where appropriate? (i.e. service calls, db calls, etc)
  • Is there any configuration information (usernames, pwds, etc) checked into code? Salesforce potentially exempt.

Documentation

  • Do all functions have a header comment?
  • Does any funky code have inline comments?
  • Is the use of third-party libs documented?
  • Are any data structures or units of measurement explained?
  • Is there any incomplete code which lacks a TODO (if TODO is allowable)?

Testing

  • Is the code testable?
  • Is there a test framework used to facilitate things like Mocking, when appropriate?
  • Do the tests appear to be comprehensive?
  • Do all tests pass?
  • Could any code under test, and its tests, be replaced with a well-tested API?
  • If applicable, are there written manual tests which appear comprehensive?

C#/.NET Code

  • If applicable, is LINQ being used instead of other approaches?
  • Is there any utility code which would work out better semantically in an extension method?
  • For all publicly-accessible functions, are XML comments used, with all parameters and return values documented?
  • For C#5/.NET 4.5 projects, are async/await constructs used where asynchronous code is written?

JavaScript Code

TODO

Python Code

OSL and their python-slingers should write some stuff up in here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment