Skip to content

Instantly share code, notes, and snippets.

@wgh000
Forked from nerandell/code-review-checklist.md
Last active August 18, 2019 12:37
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save wgh000/c6893648d51e179badb3f28f7c7a5fcf to your computer and use it in GitHub Desktop.
Save wgh000/c6893648d51e179badb3f28f7c7a5fcf to your computer and use it in GitHub Desktop.
PHP Code Review Guidelines

Make sure these boxes are checked before submitting the PR

General

  • The code works
  • The code is easy to understand
  • Follows coding conventions PSR-1 and PSR-2
  • Declared variables are used
  • Variable names are simple, short and spelt correctly
  • No negatively named boolean variables
  • Variable names contain units where applicable
  • Variables are in the smallest scope possible
  • There are no usages of magic numbers (see also)
  • There is no code that can be replaced with library functions
  • Code is not repeated or duplicated (https://en.wikipedia.org/wiki/Don%27t_repeat_yourself)
  • “Dead Code” is removed or, if temporary hack, it should be identified as such
  • Exceptions are not eaten if caught, unless explicitly documented otherwise
  • == operator and === (and its inverse !==) are not mixed up
  • Try to use constants in the left-hand side of the comparison (2 == $variable)
  • Always try using single quote (') instead of double quotes (") when working with strings
  • Floating point numbers are not compared for equality
  • Blocks of code inside loops are as small as possible and have correct termination conditions
  • Methods return early without compromising code readability
  • Performance is considered

Architecture

  • Design patterns if correctly applied
  • A class should have only a single responsibility
  • Classes, modules, functions, etc. should be open for extension, but closed for modification
  • Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program
  • Many client-specific interfaces are better than one general-purpose interface

API

  • APIs and other public contracts check input values and fail fast
  • API checks for correct oauth scope / user permissions
  • Any API change should be reflected in the API documentation
  • APIs return correct status codes in responses

Logging

  • Logging should be easily discoverable
  • Required logs are present
  • Frivolous logs are absent
  • No print_r, var_dump or similar calls exist

Documentation

  • Comments should indicate WHY rather that WHAT the code is doing
  • All methods are commented in clear language
  • Comments exist and describe rationale or reasons for decisions in code (link task ID where possible)
  • All public methods/interfaces/contracts are commented with describing usage
  • All edge cases are described in comments
  • All unusual behaviour or edge case handling is commented
  • Data structures and units of measurement are explained

Security

  • All data inputs are checked (for the correct type, length/size, format, and range)
  • No sensitive information is logged or visible in a stacktrace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment