captainhook
captainhook copied to clipboard
If php-parallel-lint is installed, the shipped "Linting" action should use it
Currently, the CaptainHook\App\Hook\PHP\Action\Linting action uses
$result = $process->run($this->php . ' -l ' . escapeshellarg($file));
As this is called once per file, it can be slow for large commits. If the project already contains php-parallel-lint, the action could use this to lint all files in a single execution in parallel (and fallback to the current implementation if the package is not installed).
It should be simple and cheap to check for installed packages via \Composer\InstalledVersions::isInstalled('foo/bar') (see https://getcomposer.org/doc/07-runtime.md#installed-versions).
Since php -l also accepts multiple arguments, one could pass all files instead of doing a single call per file. In my test, the difference was that php -l a.php b.php will not fail early. So even if a.php is invalid, it will check b.php ...
I think I would prefer a separate Action for parallel linting. I don't like the idea of using Composer to detect if something is installed. That would complicate the PHAR build an introduce dependencies to composer.json and so on.
If you can detect it without using Composer I'm all for it. An alternative could be to add an Option to the Action (eg linter => path/to/php-parallel-lint) so it becomes an active choice.
But I think I would prefer the own Action strategy if you can't reliably detect it without using Composer classes.
or use this
{
"action": "vendor/bin/php-parallel-lint {$CHANGED_FILES}"
}