-
Notifications
You must be signed in to change notification settings - Fork 55
Open
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
CIenv 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;
}
Copilot