Checklist de code review do Magento 2 para pull requests

Rodrigo Biazzi

5 de março de 2026

O code review em um projeto Magento precisa pegar mais do que problemas de estilo. Um pull request com aparência limpa ainda pode desabilitar o full page cache do catálogo inteiro, rodar uma query por produto em uma listagem ou abrir uma brecha de XSS em um template. A maioria desses problemas fica invisível em um diff se você não souber onde olhar.

Este é o checklist que uso ao revisar código Magento 2. Ele assume que as partes chatas estão automatizadas, para que o review possa focar no que as ferramentas não pegam.

Antes de ler o código

Automatize tudo que pode ser automatizado. O revisor não deveria estar apontando indentação ou type hints faltando. No CI, antes de alguém olhar o pull request:

PHPCS com o standard Magento2 nos arquivos alterados, veja o artigo sobre PHPCS
PHPStan em um nível combinado, veja o artigo sobre PHPStan
Testes unitários e, onde existirem, testes de integração
setup:di:compile, que pega argumentos de construtor quebrados e classes faltando

Depois leia a descrição. Um bom pull request diz o que muda, por quê e como testar. Se a descrição não existe, peça primeiro. Revisar código sem saber o objetivo é adivinhar.

Arquitetura e DI

Sem ObjectManager direto. ObjectManager::getInstance() em um módulo esconde dependências e atrapalha os testes. Peça injeção por construtor.
Interfaces em vez de classes concretas. Dependa de ProductRepositoryInterface, não do resource model do produto, quando o repository resolve.
O ponto de extensão certo. Uma preference que substitui uma classe inteira do core para mudar um método é sinal de alerta. Um plugin ou um observer geralmente basta. Os prós e contras estão em plugins vs observers vs preferences.
Plugins around só quando necessário. Um plugin around que sempre chama $proceed e altera o resultado deveria ser um plugin after. Plugins around deixam os stack traces mais longos e são mais fáceis de quebrar.
Sem lógica em construtores. Construtores atribuem dependências e nada mais. Trabalho em um construtor roda toda vez que o objeto é criado, inclusive durante a compilação e em requests sem relação.
Dependências pesadas atrás de proxies. Comandos CLI e plugins em classes comuns que injetam algo caro devem usar um proxy no di.xml.
DI por área. Um plugin que só importa na storefront pertence a etc/frontend/di.xml, não ao global.

Performance

É aqui que se escondem os erros mais caros.

Loads dentro de loops

foreach ($order->getAllItems() as $item) {
    $product = $this->productRepository->getById($item->getProductId());
    $brands[] = $product->getData('brand');
}

Um load por item, cada um com todos os atributos. Peça uma única collection só com o atributo necessário e um filtro pelos IDs, ou que o dado seja lido do order item se já estiver lá.

Collections sem limite

Uma collection sem page size, usada em um cron ou em uma ação do admin, funciona com cinquenta registros em desenvolvimento e estoura a memória com cinquenta mil em produção. Peça batching com setPageSize() e setCurPage(), ou uma query que leia só os IDs.

Full page cache

Procure no diff por cacheable="false" no layout XML. Um único block com esse atributo desabilita o full page cache de toda página que usa o handle. Em default.xml ou catalog_product_view.xml isso significa a loja inteira. Conteúdo específico do cliente deve vir de customer data sections ou de uma request AJAX separada.

Observers em eventos quentes

Um observer em catalog_product_load_after ou controller_action_predispatch roda o tempo todo. Confira o que ele faz. Se carrega dados ou chama uma API externa, precisa de um retorno antecipado barato ou de outro evento.

Chamadas externas dentro da request

Uma chamada HTTP a um ERP ou a uma API de frete durante o checkout deixa o checkout tão lento e tão pouco confiável quanto essa API. Peça timeouts, tratamento de erro e uma queue quando o resultado não é necessário imediatamente.

Segurança

Escaping em templates. Todo valor dinâmico em um arquivo .phtml passa por $escaper: escapeHtml, escapeHtmlAttr, escapeUrl ou escapeJs conforme o contexto. Um echo $block->getSomething() simples é um apontamento.
SQL com valores vinculados. Os valores passam por placeholders ou quoteInto, nunca por concatenação de string.
ACL em controllers do admin. Controllers do admin definem ADMIN_RESOURCE com um resource declarado em acl.xml. ACL ausente ou genérica significa que qualquer usuário do admin pode executar a ação.
Form keys e métodos HTTP. Controllers que alteram dados implementam HttpPostActionInterface e validam o form key.
Permissões da Web API. Um endpoint novo em webapi.xml com acesso anonymous precisa de um motivo muito bom. Confira se os endpoints de cliente usam self e leem o ID do cliente do token, não da request.
Segredos. Chaves de API pertencem à config criptografada ou ao env.php, nunca ao código nem a um config.xml em texto puro.
Logs. Logs não devem conter dados de cartão, senhas, tokens nem registros completos de clientes.

Um pequeno exemplo do ponto sobre SQL:

$select = $connection->select()
    ->from($this->resource->getTableName('sales_order'), ['entity_id'])
    ->where('customer_email = ?', $email)
    ->where('created_at >= ?', $from);

Mudanças de banco e de dados

Declarative schema. Mudanças de schema vão em db_schema.xml com um db_schema_whitelist.json atualizado. Sem novos scripts InstallSchema ou UpgradeSchema. Veja declarative schema e data patches.
Data patches são idempotentes. Rodar um patch duas vezes, ou em uma loja onde o dado já existe, não pode falhar nem duplicar dados.
Índices para queries novas. Um filtro novo em uma tabela grande precisa de índice, e um índice novo em uma tabela grande precisa de um plano para o tempo de deploy.
Mudanças retrocompatíveis. Remover ou renomear uma coluna enquanto o código antigo ainda pode estar rodando quebra rollbacks e deploys sem downtime.

Configuração

Novas configurações têm defaults no config.xml e as flags showInDefault, showInWebsite e showInStore certas no system.xml.
O código lê a configuração no escopo certo, geralmente o escopo de store na storefront.
As mudanças no app/etc/config.php casam com as mudanças nos módulos. Um módulo novo que não está no config.php não será habilitado em produção.
Valores específicos de ambiente não são commitados no config.php.

Frontend

Overrides de template copiam só o necessário. Copiar um template grande do core para mudar uma linha significa perder toda correção futura dele. Uma mudança de layout, um plugin no block ou um pequeno template filho geralmente é melhor.
Mudanças de JavaScript em componentes do core usam mixins do RequireJS em vez de substituir arquivos inteiros.
Novos JS e CSS são carregados só nas páginas que precisam, não no default.xml.
Textos mostrados ao cliente são traduzíveis com __() em PHP e $t em JS.
Em projetos Hyva, os componentes Alpine seguem as convenções do tema e não puxam código baseado em jQuery ou RequireJS.

Testes e impacto no deploy

A lógica de negócio tem testes unitários. A ligação que importa, como um plugin que altera um preço, tem um teste de integração ou um plano de teste manual claro.
O pull request diz se precisa de setup:upgrade, reindex, cache flush ou restart de queue consumer. Isso muda como ele é publicado.
Novos cron jobs e consumers são documentados com seu agendamento e tempo de execução esperado.
Módulos ou recursos removidos têm um plano para os dados e configurações que sobram.

Como dar o feedback

Separe o que bloqueia das sugestões. Problemas de segurança, risco de perda de dados e regressões de performance bloqueiam. Preferências de nomenclatura não. Explique por que algo é um problema com um cenário concreto, como o que acontece com um catálogo grande ou com uma segunda store view. E quando o código está bom, diga. Um review que só lista problemas ensina as pessoas a temer o próximo.

FAQ

O que deve ser automatizado em vez de revisado na mão?

Coding standard, análise estática e testes. Rode o PHPCS com o standard Magento2, o PHPStan e os testes unitários no CI, para que o revisor gaste tempo com design, performance e segurança.

Qual o problema de performance mais comum do Magento em pull requests?

Carregar models ou collections dentro de loops e cacheable=false em um handle de layout usado em muitas páginas. Os dois parecem inofensivos em um diff pequeno e doem muito em produção.

Usar o ObjectManager diretamente é sempre errado?

No código de aplicação, sim, com pouquíssimas exceções, como algumas factories e mudanças retrocompatíveis de construtor no core. Em um módulo, peça injeção por construtor.

Qual deve ser o tamanho de um pull request no Magento?

Pequeno o bastante para o revisor entender cada arquivo. Uma feature ou correção por pull request, com mudanças de schema, configuração e código juntas, para que a mudança possa ser publicada e revertida como uma unidade.

Atendo como freelancer em Magento e Adobe Commerce, incluindo code reviews e auditorias de módulos existentes. Se precisar de ajuda com isso, fale comigo.