Code review has been an integral part of my career as a developer. Whether it in the beginning through reviewing themes for WordPress.org, reviewing client code at WordPress.com VIP, or now when reviewing code from peers as a Principal Engineer at Human Made.

Code review has both allowed me to learn and grow as an engineer, as well as teach and mentor other developers in return.

There is an ever growing number of issues that I look out for. This article is just a list of all these things, and the reference I use for reviews.

Documentation

  1. If this a new plugin, or an architectural modification: Does the plugin have an up to date README.md in the root directory?
  2. Do all files have file header documentation?
  3. Do all code constructs (classes, functions, and hooks) have complete and correct documentation?

PHP

Code Style

  1. Do all functions have clear names?
  2. Are early returns used if possible?
  3. Are there functions returning HTML? If so, they need to output the HTML instead.
  4. Are there no functions constructing creating lots of HTML? If so, they need to use template parts.
  5. Are there no functions directly outputting inline CSS or JS? If so, they need to either use dedicated files and enqueue them, or use the corresponding helpers for inline code.

Error Handling

  1. Do all new functions use exceptions instead of WP_Error objects if possible? See Why WP_Error Sucks for more information.
  2. Are grave errors logged?

Namespaces, Naming, and Organisation

  1. There should be no use function statements.
  2. Are all namespaces using boostrap() functions for adding hooks?
  3. Is the main bootstrap function not hooked into plugins_loaded?

Strict Typing

  1. Is there a declare( strict_types=1 ); statement at the top of the file?
  2. Do all PHP functions have type hinting for arguments and returns if possible?
  3. Are null returns used to signify an acceptable failure case if possible?

WordPress APIs

  1. Do all the functions use WordPress objects if possible?
  2. Are all custom hook calls (do_action and do_filter) wrapped in try/catch statements? This is to prevent exceptions from not being caught.
  3. Are there no anonymous functions used as hook callbacks? If so they must be moved to named functions.

Static Assets

  1. Were the version numbers of modified assets bumped to clear caches?
  2. If new libraries are added: are these not already used in the project?