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

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

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

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

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

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

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

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

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

  • 根本的な原因を特定することで、コードレビューに必要な過度の労力に関する問題に対処します。 通常、その理由は、要件が開発チケットに十分に翻訳されていない(コミュニケーションの問題、ユーザーのストーリーの不備が原因)か、コーチングの問題であるかのいずれかです。 最初のケースでは、チケットに、チームメンバーが要件を効率的に配信するのに十分な情報が含まれていることを確認することをお勧めします。 2 つ目のケースでは、チーム構造を調整して、より多くのシニアエンジニアを含めるか、メンタリングやコーチングの外部で承認を得る必要がある場合があります。

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

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

  • クラウドインフラストラクチャー上のAdobe Commerce
  • Adobe Commerce オンプレミス

コードレビューで探すべき内容

スタイル

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

必ず PHPMD と PHPCS を設定し、CLI (これも後述)から コーディング標準ツールを実行します。 重複する部分もありますが、両方とも一意のテストを実施しています。

規則と構造

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

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

完全性

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

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

パフォーマンス

パフォーマンスのレビューは手動で行いますが、不確かな場合はコード実行で支援できます。

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

セキュリティ

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

  • 必要な場合、例外はログに記録されますか。 使用される例外のタイプが適切かどうか。
  • around プラグインは避けられますか?
  • プラグインは正しいタイプのデータを返しますか?
  • データベース抽象化レイヤーを使用して作成する必要がある生の SQL クエリを見つけることができますか。
  • 新しいタイプのデータは、あらゆるタイプのユーザー、管理者またはフロントエンドに公開されますか? そのリスクはセキュリティリスクですか?
  • ユーザー生成データは検証されますか? cookie の値やサーバーヘッダーなど、ブラウザーから送信されるすべての情報は、ユーザー生成と見なされます。

プライバシーと GDPR

プライバシーおよび GDPR のレビューは手動で行います。

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

メンタリング

メンタリングのレビューは手動で行われます。

  • チームを教育することを目的として、知識を共有する場所を探します。
  • コメントが単なる教育的なものであっても、標準を満たすことが重要ではない場合は、作成者が解決する必要がないことを示します。
  • 何かいいものが表示された場合は、特にコメントの 1 つに対して優れた方法で対処した場合に、開発者に伝えてください。 コードレビューは、多くの場合、誤りに焦点を当てますが、好事例に対する励ましや評価も提供する必要があります。 何が間違っていたかを開発者に伝えるよりも、開発者に何が正しかったかを伝えることが、メンタリングの面で役立つ場合があります。

自動処理

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

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

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

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

    code language-bash
    #!/usr/bin/env bash
    $HOME/web/magento/magento-coding-standard/vendor/bin/phpcs --standard=Magento2 "$@"
    
  • プスタン

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

  • PhpStorm PHP のコピー/ペースト検出

recommendation-more-help
754cbbf3-3a3c-4af3-b6ce-9d34390f3a60