Code review checklist #

This page is a checklist for reviewing a change to Exponential or one of its extensions. The project does many review rounds before it accepts a change (Contributing upstream); a shared list makes each round shorter. Pair it with the Security review checklist.

Contents #

1. Scope and history
2. Architecture
3. Compatibility
4. Runtime
5. Tests and CI
6. Settings, templates and styling
7. Documentation and release record

Scope and history #

• To do: One change per pull request: a fix and a refactoring are two.
• To do: Commit subjects start with Added: , Updated: , Fixed: , Removed: or Renamed: ; the body says why and what (Git workflow).
• To do: The product is named Exponential; kernel identifiers such as eZContentObject stay as they are.
• To do: No tool attribution, private paths, credentials or machine-specific settings in commits.
• To do: The branch is squashed into logical commits and rebased onto main while open.

Architecture #

• To do: The code is in the right layer (Kernel, lib and extensions: the layering): no new lib/ dependency on kernel/, no HTML printed by a domain class.
• To do: New code uses the exp prefix; nothing existing is renamed.
• To do: An extension point is used before a kernel override (Events, extension points and plugins).
• To do: Shared logic is a service class, not copied between a view, a command and a cronjob part.
• To do: Settings are read through eZINI; debug output through eZDebug; text through ezpI18n::tr().
• To do: New classes have their class maps regenerated.

Compatibility #

• To do: Runs on PHP 8.0: no newer syntax or functions without a guard; no PHP 7+ type declarations in kernel code (Static analysis and code style).
• To do: SQL works on every supported database; schema changes ship for every driver (Schema changes and migrations).
• To do: No public signature, setting, template variable, command option or exit code changed without upgrade notes (Backward compatibility policy).
• To do: New behavior is opt-in, or a safe default that keeps the old result.

Runtime #

• To do: Correct in a long-lived Velocity worker: no per-request state in statics, idempotent registrations, no exit (Concurrency, statics and long-lived processes).
• To do: A new class named in a shipped INI file is guarded with class_exists().
• To do: Cache files are written through eZClusterFileHandler; caches have ids and tags and are invalidated (Cache invalidation and clearing).
• To do: Commands keep --help and exit codes stable and offer --dry-run when they change data.
• To do: Performance: no query in a loop where one query does; measure when in doubt (Performance measurement and benchmarks).

Tests and CI #

• To do: A test fails without the change and passes with it.
• To do: Tests need no database unless they test SQL, and read nothing from a developer's installation.
• To do: The tests are in a suite of phpunit.xml, or they never run.
• To do: Quality and PHPUnit are green on the merge with main (CI gates).
• To do: share/filelist.md5 has the checksum of every added, changed or removed file (make manifest-fix).

Settings, templates and styling #

• To do: A new setting has its default and a comment in settings/<file>.ini; switches use enabled/disabled.
• To do: Templates escape output and contain no SQL.
• To do: Administration templates follow Styling the administration: no <style> or style="", shared components and tokens, a dark value for every new token, module CSS in stylesheets/modules/.
• To do: Changed pages checked in light and dark mode and at the widths the administration supports.

Documentation and release record #

• To do: The feature page in doc/eng-us/features/6.0/ is written or updated.
• To do: The changelog lists the change (Changelogs and upgrade notes).
• To do: A change that needs action from installations has a page in doc/eng-us/bc/6.0/.
• To do: Help pages touched by the change pass exp:help check (Documentation as code).

Related pages #

• Design review checklist
• Pull request etiquette
• Coding standards
• Free documentation: this page is free to copy, translate and improve under the GNU FDL 1.3.