Code Review Checklist for Magento 2 Pull Requests

1. Why Magento pull requests need their own checklist

A generic PHP code review checks readability, naming conventions and obvious bugs, but misses the traps that are specific to Magento. A preference that overrides a core class looks harmless in the diff, but can conflict with any other module trying to override the same class. A code review without Magento context lets such conflicts through until they surface during integration with a third-party module.

A Magento-specific code review checklist adds points to the generic quality criteria that only matter within this framework: declarative schema instead of imperative install scripts, plugin sort order, backward compatibility boundaries of the public API, and typical performance traps like N+1 queries in collections. These points can be cast into a fixed checklist that every team member consistently goes through on every pull request.

This article defines exactly that checklist, point by point, with concrete diff examples for each category. The goal is a code review process that reliably catches Magento-specific mistakes before merge, instead of discovering them in production.

2. Checking declarative schema instead of install scripts

The first point on every Magento code review checklist: does the PR add a new InstallSchema.php or UpgradeSchema.php, even though declarative schema via db_schema.xml has been the recommended path since Magento 2.3? Imperative scripts are harder to diff, run only once, and leave no declarative source of truth for the current table state. A reviewer should question every new scripts folder as to why declarative schema is not sufficient here.

The second point: after a change to db_schema.xml, was the corresponding db_schema_whitelist.json also updated? This file is generated by bin/magento setup:db-declaration:generate-whitelist and must be committed in the same PR, otherwise the schema change will not be applied on the next setup:upgrade in another environment. This point of the code review checklist is, by experience, the one most frequently forgotten.

<!-- app/code/Vendor/Module/etc/db_schema.xml -->
<!-- Review checklist: does this table addition have a matching whitelist entry? -->
<schema xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:Setup/Declaration/Schema/etc/schema.xsd">
    <table name="vendor_custom_entity" resource="default" engine="innodb" comment="Custom Entity">
        <column xsi:type="int" name="entity_id" padding="10" unsigned="true" nullable="false"
                identity="true" comment="Entity ID"/>
        <column xsi:type="varchar" name="code" nullable="false" length="64" comment="Unique Code"/>
        <constraint xsi:type="primary" referenceId="PRIMARY">
            <column name="entity_id"/>
        </constraint>
        <constraint xsi:type="unique" referenceId="VENDOR_CUSTOM_ENTITY_CODE">
            <column name="code"/>
        </constraint>
    </table>
</schema>

3. Spotting plugins vs. preferences in the diff

One of the most important checks in any code review checklist: does the PR replace a core class via preference in di.xml, even though a plugin would achieve the same effect? Preferences allow exactly one implementation per interface, while plugins can be registered independently by multiple modules. A reviewer who sees a new preference should always ask whether a before, after or around plugin would be sufficient instead.

For plugins themselves, sort order (sortOrder) is the second critical point. Two plugins on the same method without explicit sortOrder values behave non-deterministically depending on module load order. A good code review checklist requires that every new plugin sets an explicit sortOrder and justifies in the PR comment why this order was chosen relative to existing plugins.

<?php
declare(strict_types=1);

// WRONG in review: preference replaces the entire class,
// blocking any other module from customizing the same behavior
// <preference for="Magento\Catalog\Model\Product" type="Vendor\Module\Model\Product" />

// RIGHT: plugin targets only the specific method, coexists with other modules
namespace Vendor\Module\Plugin;

use Magento\Catalog\Model\Product;

/**
 * Adjusts product price display for custom business logic.
 */
class ProductPricePlugin
{
    /**
     * Applies a surcharge to the final price for flagged products.
     *
     * @param Product $subject Product being processed
     * @param float $result Original price
     * @return float Adjusted price
     */
    public function afterGetFinalPrice(Product $subject, float $result): float
    {
        if ($subject->getData('requires_surcharge')) {
            return $result * 1.05;
        }
        return $result;
    }
}

4. Backward compatibility and public API boundaries

Magento marks interfaces and classes in the Api namespace, as well as those annotated @api, as public API subject to a backward compatibility guarantee. A code review must check whether a method signature in such a class was changed, a parameter added, or a return type tightened, since all of these are breaking changes for third-party modules developing against this API.

A common mistake in pull requests: a new mandatory parameter is added to an existing public method without a default value. This compiles fine locally but breaks every external implementation that calls or overrides this method. At this point, the code review checklist should explicitly require that new parameters on public interfaces always get a default value, or are introduced via a new, separate interface.

Removing code marked @deprecated also belongs in this check: Magento's own policy requires at least two minor versions of transition time before deprecated code may actually be removed. A PR that falls short of this deadline should be rejected in review, regardless of how clean the rest of the code is.

5. Performance red flags in code review

Certain code patterns keep showing up in Magento pull requests and are almost always a performance problem: Collection::load() inside a loop generates a new database query per iteration, instead of filtering once on the collection before the loop. A reviewer who spots a collection instantiation inside a foreach should flag it as a clear N+1 candidate.

A second red flag for the code review checklist: direct ObjectManager::getInstance() calls outside of factories or test bootstrapping. This bypasses dependency injection entirely, makes the class untestable, and hides real dependencies. Every finding should lead to a mandatory switch to constructor injection before the PR is merged.

<?php
declare(strict_types=1);

// WRONG: N+1 query pattern flagged in review
foreach ($orderIds as $orderId) {
    $order = $this->orderCollectionFactory->create()
        ->addFieldToFilter('entity_id', $orderId)
        ->getFirstItem();
    // process $order
}

// RIGHT: single collection query, filtered once with an array
$orders = $this->orderCollectionFactory->create()
    ->addFieldToFilter('entity_id', ['in' => $orderIds]);
foreach ($orders as $order) {
    // process $order
}

6. Security checks: SQL injection, XSS, ACL

SQL injection risks in Magento almost always arise from string concatenation in direct SQL calls instead of using the query builder with bound parameters. A code review checklist must treat every call to getConnection()->query() with directly embedded variables as a critical finding and require switching to bind parameters or the Zend Framework query builder.

In phtml templates, unescaped output of user input is the most common XSS vector. Magento's $escaper view model provides escapeHtml(), escapeUrl() and escapeJs() for exactly this purpose, and a review should reject any direct echo or shorthand output without an escaping call. In addition, ACL checks belong in every admin controller: if an _isAllowed() override or a matching adminhtml_acl entry is missing, any admin user can access the new functionality regardless of their role.

<?php
declare(strict_types=1);

namespace Vendor\Module\Controller\Adminhtml\Report;

use Magento\Backend\App\Action;

/**
 * Displays the custom report grid, restricted to a dedicated ACL resource.
 */
class Index extends Action
{
    /**
     * ACL resource required to access this controller.
     */
    public const ADMIN_RESOURCE = 'Vendor_Module::custom_report';

    /**
     * Checks whether the current admin user is allowed to view this page.
     *
     * @return bool
     */
    protected function _isAllowed(): bool
    {
        return $this->_authorization->isAllowed(self::ADMIN_RESOURCE);
    }
}

7. Coding standards and static analysis in the PR

The Magento coding standard (magento/magento-coding-standard) and PHPStan at least at level 5 belong as an automated pre-check before every manual code review. A reviewer should never spend time on formatting discussions that a CI check could already enforce automatically. The code review checklist should therefore explicitly require that bin/phpcs and bin/analyse pass without errors before the PR is even released for manual review.

For teams that have not yet automated these checks in CI, a pre-merge gate with GitHub Actions or GitLab CI is the most pragmatic first step. The manual reviewer can then focus entirely on the Magento-specific points of this checklist instead of manually checking indentation and naming conventions.

#!/usr/bin/env bash
# Pre-review automated gate — run before requesting a manual code review
set -euo pipefail

echo "[1/3] Magento Coding Standard"
bin/phpcs --standard=Magento2 app/code/Vendor/Module

echo "[2/3] PHPStan static analysis"
bin/analyse app/code/Vendor/Module --level=5

echo "[3/3] Declarative schema whitelist check"
bin/magento setup:db-declaration:generate-whitelist --module-name=Vendor_Module --dry-run

echo "[OK] All automated checks passed — ready for manual review"

8. Test coverage and PR size

A pull request without unit or integration tests for new business logic should automatically be considered incomplete in any code review checklist, regardless of how good the rest of the code looks. Plugins and observers in particular, which change production-critical behavior, need at least one test documenting the expected behavior before and after the change.

PR size itself is an often underestimated review factor. A pull request with over five hundred changed lines is, in practice, read thoroughly far less often than a focused PR under a hundred lines. Teams developing large features should split them into several smaller, independently reviewable and mergeable pull requests instead of producing a single massive review package that ends up only superficially checked.

9. Review categories and tools at a glance

The following table maps each category of the code review checklist to the appropriate tool or check step.

The more points of the code review checklist run automated in CI, the more time the reviewer has for the points that strictly require human judgment: architecture decisions, backward compatibility risks and security questions that no linter reliably detects.

10. Summary

A resilient code review checklist for Magento 2 pull requests covers four categories that a generic PHP review misses: declarative schema instead of install scripts, plugin usage instead of preferences with correct sort order, backward compatibility boundaries of the public API, and performance red flags like collection queries in loops. Complemented by security checks for SQL injection, XSS and ACL, this becomes a complete inspection.

The biggest lever lies in automation: anything a CI gate with phpcs, PHPStan and the whitelist check can reliably verify should never be discussed manually in review. That creates room for the points of the code review checklist that actually require human judgment, above all architecture decisions and backward compatibility risks.

11. FAQ: Code Review Checklist for Magento 2 Pull Requests