Adobe Commerceのコードレビューのベストプラクティス

コードレビューの主な目的は、実装された機能が、パフォーマンス、アーキテクチャ、セキュリティの観点から、最適な方法で要件を満たしているかどうかを検証することです。

さらに、コードレビューは、コードがAdobe Commerce開発のベストプラクティスを満たし、理解しやすく維持しやすく、コーディング標準に準拠していることを確認することを目的としています。 ほとんどのコーディング標準は、適切なツールによって自動的にチェックする必要があります。

推奨されるコードレビュープロセス

コードレビュープロセスは、大まかに言えば、次の手順で構成されます。

  1. ローカル開発環境のチェックアウト機能ブランチ。
  2. コードがレビュー用に送信されてから時間が経過している場合は、ターゲットブランチ(通常はQA ブランチ)から最新の変更をマージし、リモート機能ブランチ(存在する場合)に更新をプッシュします。
  3. 上位レベルのレビューを実行して、コードがすべての要件を実装していることを検証します。 大きな相違がある場合は、開発者に連絡して説明を求めます。
  4. コードの実行はオプションですが、コードが期待どおりに動作するか疑問がある場合、またはプロセスの効率性が向上する場合は、実装された機能が主要な使用シナリオで期待どおりに動作するかどうかをテストします。 大きな問題がある場合は、レビュープロセスを停止し、適切なコメントを付けてチケットを再度開きます。
  5. コードがすべての要件を実装していることを確認するには、徹底的なレビューを行います。 問題がある場合は、プルリクエストに適切なコメントを追加し、チケットを再度開きます。
  6. 問題がなければ、プルリクエストをマージします(または、プルリクエストが確立されている場合は、次のコードレビューレベルに渡します)。

また、コードレビュープロセスを実装する際には、次の点を考慮してください。

  • コードレビューは、チーム内でのコミュニケーションと知識の伝達のための主要なツールです。 プルリクエストに大きな問題が含まれておらず、必要なビジネスロジックが実装されている場合でも、マージ後にさらに改善を提案する機会として使用できます。

  • 平均すると、コードレビューに10%~20%以上の開発作業を費やすべきではありません。 これは、開発チームがうまく連携するシニアエンジニアで構成されることを前提としています。 コードレビューに時間がかかると、プロジェクトの予算やタイムラインに影響を与える可能性があります。

  • コードレビューに必要な過度の労力に関する問題は、根本原因を特定することで対処できます。 これは通常、要件が(コミュニケーションの問題やユーザーストーリーの不足により)開発チケットに変換されにくいか、コーチングの問題のいずれかであることが原因です。 1つ目のケースでは、推奨される緩和策は、チームメンバーが要件を効率的に提供するのに十分な情報がチケットに含まれていることを確認することです。 2つ目のケースでは、上級管理職を含めるか、メンタリングやコーチング以外の分野で承認を得るために、チーム構造を調整する必要があるかもしれません。

影響を受ける製品とバージョン

​ サポートされているすべてのバージョン ​ /:

  • Adobe Commerce on cloud infrastructure
  • Adobe Commerce オンプレミス

コードレビューに求められるもの

Style

スタイルは、PhpStorm検査を実行することで自動的にテストできます(以下を参照)。

PHPMDおよびPHPCSを設定し、CLIから​ コーディング標準 ツールを実行してください(以下も参照)。 重複もありますが、それぞれ固有のテストもあります。

条約と構造

規則と構造のレビューは手動で行われます。

  • クラスの機能は単一の責任に限定されますか?
  • ディレクトリ構造は意味がありますか?
  • 適切なレベル(サーバー、クライアント、CSS、JS、データベース、フレームワーク、インフラストラクチャ)で実行される機能です。
  • バージョン管理は正しいですか?
  • コードが型破りに見えたり、間違った方法で問題を回避しようとしているように見えたりしますか?
  • モジュール名、パッケージ名、リポジトリ名の命名規則が正しく適用されていますか?
  • グローバル CSS スタイルが慎重に適用され、過剰に使用されていないことを確認します。

完全性

完全性のレビューは手動で行われます。

  • コードは設定によって有効または無効にでき、必要なすべてのコードが期待どおりに動作しますか?
  • チケットに記載されているすべての設定は存在しますか? 範囲、データタイプ、検証、翻訳、デフォルト値を確認します。
  • 設定は、常に可能な限り低いレベル(ストアビューレベル、web サイトレベル、グローバルレベル)で取得されますか? 設定取得は、system.xml ファイルのスコープの定義と一致する必要があります。
  • 技術仕様のフローチャートのすべてのパスが対象ですか? その他の技術的な仕様はすべてカバーされていますか?
  • 新しい機能に対してACLが定義されていますか?
  • PhpDocsは明確ですか? コミットメッセージは明確ですか?
  • 何かコードがコメントされているか、それともデバッグコードが表示されているか。

パフォーマンス

パフォーマンスのレビューは手動で行われるため、不明な場合はコード実行でサポートできます。

  • クエリはループで実行されますか? このループは、編集したファイルの外部に存在する可能性があります。
  • cachable="false"属性を見つけることができますか? 正しく適用されているか?

セキュリティ

セキュリティに関するレビューは手動で行われ、テキスト検索でサポートできます。 セキュリティチェックの一部は、自動テストによって処理されます。

  • 必要に応じて例外をログに記録しますか? 適切な種類の例外を使用していますか?
  • around個のプラグインを避けることはできますか?
  • プラグインは正しいタイプのデータを返しますか?
  • データベース抽象化層を使用して構築する必要がある生のSQL クエリを見つけることができますか?
  • どのようなタイプのユーザー、管理者、フロントエンドにも新しいタイプのデータが公開されますか? その露出はセキュリティリスクですか?
  • ユーザー生成データは検証されますか? ブラウザーから提供されるすべての情報は、Cookie値やサーバーヘッダーを含め、ユーザーが生成したものとみなされます。

プライバシーとGDPR

プライバシーとGDPRのレビューは手動で行われます。

  • コードは顧客データやメールを処理しますか? 特に注意を払う。
  • このコードをループで実行できる場合、あるループサイクルから別のループサイクルに顧客データをリークできますか?
  • リスクの指標は、インポート、cron ジョブ、トランザクションメール、バッチキューハンドラーです。
  • ループ内のユーザーデータを分離します。 Adobeでは、ファクトリやリポジトリを使用して、ループ外ではアクセスできないループサイクルでモデルを作成することをお勧めします。

指導

メンタリングのレビューは手作業でおこなわれます。

  • チームを教育することを目的として、知識を共有する場所を探します。
  • コメントが純粋に教育的であるが、基準を満たすために重要ではない場合は、作成者がコメントを解決する必要がないことを示します。
  • 何か良いものを見たら、開発者に伝えてください。特に、コメントに適切に対応した際に重要です。 コードレビューでは、多くの場合、失敗に焦点を当てますが、優れた慣行に対する奨励と感謝の気持ちを示す必要があります。 時には、開発者に対して、彼らが正しいことをしたと言うよりも、彼らが正しいことをしたと言う方が、メンタリングという点で、さらに価値があることもあります。

自動化

開発者は、自動化を使用して、DIのコンパイル、データベーススキーマ、コンポーザーの検証、コーディング標準の遵守を確認できます。

  • DI コンパイル – 次のCLI コマンドを実行して、コードを問題なくコンパイルできるかどうかを確認します。

    code language-shell
    bin/magento module:disable -n -q --all || exit;
    bin/magento module:enable -n -q --all || exit;
    bin/magento cache:enable -n -q || exit;
    bin/magento cache:flush -n -q || exit;
    rm -rf generated/*
    rm -rf var/view_preprocessed/*
    rm -rf pub/static/*
    rm -rf var/cache/*
    bin/magento deploy:mode:set --skip-compilation production || exit;
    bin/magento setup:di:compile -vv || exit;
    bin/magento setup:static-content:deploy en_US || exit;
    bin/magento deploy:mode:set developer || exit;
    
  • データベーススキーマ whitelist.json – 次のCLI コマンドを実行し、db_schema_whitelist.json ファイルが追加または変更されていないことを検証します。

    code language-shell
    bin/magento setup:db-declaration:generate-whitelist --module-name[=MODULE-NAME]
    
  • Composer validate - composer.json ファイルを含むディレクトリで次のCLI コマンドを実行して、composer.json ファイルを検証します。

    code language-shell
    composer validate
    
  • コーディング標準 – コーディング標準ツールをインストールして実行し、モジュールに対して実行します。 次のファイルは、mcs ./app/code/Vendor/Module/を入力して任意の場所で実行できるようにする方法を示しています。

    code language-shell
    #!/usr/bin/env bash
    $HOME/web/magento/magento-coding-standard/vendor/bin/phpcs --standard=Magento2 "$@"
    
  • Phpstan

    code language-shell
    ./vendor/bin/phpstan analyze app/code/Vendor/Module
    
  • PhpStorm検査

  • PhpStorm PHPのコピー/貼り付け検出器

recommendation-more-help
commerce-operations-help-implementation-playbook