drupal-starter icon indicating copy to clipboard operation
drupal-starter copied to clipboard

Optimize phpcs CI execution and improve output clarity

Open Copilot opened this issue 2 months ago • 1 comments

The phpcs GitHub Action was slow (~2m 31s) and provided ambiguous output ("No fixable errors were found" doesn't indicate if violations exist).

Changes

Performance optimization (~50% faster)

  • Skip phpcbf in CI environments (detected via CI env var) - can't commit fixes anyway
  • Combine standards: --standard=Drupal,DrupalPractice (single invocation vs 2×N×2 nested loops)
  • Use GNU parallel with temp files for parallel execution across directories

Output clarity

  • Explicit progress: "Running phpcs to check for coding standard violations..."
  • Success: "✓ No coding standard violations found!"
  • Failure: "PHPCS found coding standard violations!" (red, with guidance)

Before:

foreach ($directories as $directory) {
  foreach ($standards as $standard) {
    foreach ($commands as $command) {
      $result = $this->_exec("cd web && ../vendor/bin/$command $directory $arguments");
    }
  }
}

After:

// Step 1: Auto-fix (local only)
if (!getenv('CI')) {
  // parallel phpcbf execution
}

// Step 2: Check
$this->say('Running phpcs to check for coding standard violations...');
// parallel phpcs execution with --halt now,fail=1

Local development unchanged - phpcbf still auto-fixes.

Original prompt

This section details on the original issue you should resolve

<issue_title>Improve phpcs CI output clarity and reduce execution time by 50%</issue_title> <issue_description>Ported from a client's project:

The phpcs GitHub Action output was ambiguous ("No fixable errors were found" doesn't indicate if violations exist) and execution time was excessive at 2m 31s.

Changes

Output clarity:

  • Added explicit status messages: "Running phpcs to check for coding standard violations..."
  • Success: "✓ No coding standard violations found!"
  • Failure: "PHPCS found coding standard violations!" (red, with guidance)

Performance optimization:

  • Skip phpcbf (auto-fixer) in CI environments - detected via CI env var
  • Combine standards: --standard=Drupal,DrupalPractice (single invocation)
  • Result: ~50% faster (~1m 15s vs 2m 31s)

Backward compatibility:

  • Local development unchanged: phpcbf still auto-fixes before checking
diff --git a/composer.lock b/composer.lock
index 08899c96c..61a3adfe3 100644
--- a/composer.lock
+++ b/composer.lock
@@ -23051,5 +23051,5 @@
     "platform-overrides": {
         "php": "8.3.20"
     },
-    "plugin-api-version": "2.6.0"
+    "plugin-api-version": "2.9.0"
 }
diff --git a/robo-components/PhpcsTrait.php b/robo-components/PhpcsTrait.php
index 4c53de5a1..f1836baea 100644
--- a/robo-components/PhpcsTrait.php
+++ b/robo-components/PhpcsTrait.php
@@ -17,15 +17,7 @@ trait PhpcsTrait {
    *   successful.
    */
   public function phpcs(): ?ResultData {
-    $standards = [
-      'Drupal',
-      'DrupalPractice',
-    ];
-
-    $commands = [
-      'phpcbf',
-      'phpcs',
-    ];
+    $standards = 'Drupal,DrupalPractice';
 
     $directories = [
       'modules/custom',
@@ -37,34 +29,49 @@ public function phpcs(): ?ResultData {
       '../scripts',
     ];
 
-    $error_code = NULL;
+    $arguments = "--standard=$standards -p --ignore=" . self::$themeName . "/dist,node_modules,.parcel-cache --colors --extensions=php,module,inc,install,test,profile,theme,css,yaml,txt,md";
 
-    // Use GNU parallel for faster execution.
-    foreach ($commands as $command) {
-      // Build all command combinations for parallel execution.
+    // Step 1: Auto-fix what can be fixed (only if not in CI).
+    // In CI, phpcbf can't commit changes, so we skip it for better performance.
+    $is_ci = !empty(getenv('CI'));
+    if (!$is_ci) {
+      $this->say('Running phpcbf to auto-fix coding standard violations...');
       $command_list = [];
       foreach ($directories as $directory) {
-        foreach ($standards as $standard) {
-          $arguments = "--standard=$standard -p --ignore=" . self::$themeName . "/dist,node_modules,.parcel-cache --colors --extensions=php,module,inc,install,test,profile,theme,css,yaml,txt,md";
-          $command_list[] = "cd web && ../vendor/bin/$command $directory $arguments";
-        }
+        // Phpcbf exits with non-zero even on success when it fixes files.
+        // We don't fail on phpcbf errors since phpcs will catch real issues.
+        $command_list[] = "cd web && ../vendor/bin/phpcbf $directory $arguments || true";
       }
 
-      // Use GNU parallel to execute all commands in parallel.
-      $commands_file = tempnam(sys_get_temp_dir(), 'phpcs_commands');
+      $commands_file = tempnam(sys_get_temp_dir(), 'phpcbf_commands');
       file_put_contents($commands_file, implode("\n", $command_list));
 
-      $result = $this->_exec("parallel -j+0 --halt now,fail=1 < $commands_file");
+      $this->_exec("parallel -j+0 < $commands_file");
       unlink($commands_file);
+    }
 
-      if (empty($error_code) && !$result->wasSuccessful()) {
-        $error_code = $result->getExitCode();
-      }
+    // Step 2: Check for remaining violations.
+    $this->say('Running phpcs to check for coding standard violations...');
+    $command_list = [];
+    foreach ($directories as $directory) {
+      $command_list[] = "cd web && ../vendor/bin/phpcs $directory $arguments";
     }
 
-    if (!empty($error_code)) {
-      return new ResultData($error_code, 'PHPCS found some issues');
+    $commands_file = tempnam(sys_get_temp_dir(), 'phpcs_commands');
+    file_put_contents($commands_file, implode("\n", $command_list));
+
+    $result = $this->_exec("parallel -j+0 --halt now,fail=1 < $commands_file");
+    unlink($commands_file);
+
+    if (!$result->wasSuccessful()) {
+      $this->say('');
+      $this->yell('PHPCS found coding standard violations!', 40, 'red');
+      $this->say('Please review the errors above and fix them.');
+      return new ResultData($result->getExitCode(), 'PHPCS found coding standard violations');
     }
+
+    $this->say('');
+    $this->say('✓ No coding standard violations found!');
     return NULL;
   }
 ```</issue_description>

<agent_instructions>Apply the same changes</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>


</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes Gizra/drupal-starter#963

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/Gizra/drupal-starter/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

Copilot avatar Jan 01 '26 06:01 Copilot