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.
Table of contents
Documentation
- If this a new plugin, or an architectural modification: Does the plugin have an up to date
README.mdin the root directory? - Do all files have file header documentation?
- Do all code constructs (classes, functions, and hooks) have complete and correct documentation?
PHP
Code Style
- Do all functions have clear names?
- Are early returns used if possible?
- Are there functions returning HTML? If so, they need to output the HTML instead.
- Are there no functions constructing creating lots of HTML? If so, they need to use template parts.
- 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
- Do all new functions use exceptions instead of
WP_Errorobjects if possible? See Why WP_Error Sucks for more information. - Are grave errors logged?
Namespaces, Naming, and Organisation
- There should be no
use functionstatements. - Are all namespaces using
boostrap()functions for adding hooks? - Is the main bootstrap function not hooked into
plugins_loaded?
Strict Typing
- Is there a
declare( strict_types=1 );statement at the top of the file? - Do all PHP functions have type hinting for arguments and returns if possible?
- Are
nullreturns used to signify an acceptable failure case if possible?
WordPress APIs
- Do all the functions use WordPress objects if possible?
- Are all custom hook calls (
do_actionanddo_filter) wrapped intry/catchstatements? This is to prevent exceptions from not being caught. - Are there no anonymous functions used as hook callbacks? If so they must be moved to named functions.
Static Assets
- Were the version numbers of modified assets bumped to clear caches?
- If new libraries are added: are these not already used in the project?