PHPCS Configuration management #1
Conversation
| /** | ||
| * @var Composer | ||
| */ | ||
| protected $composer; |
There was a problem hiding this comment.
If you do not plan on having the Plugin class extended, these properties might as well be private.
| * @throws LogicException | ||
| * @throws ProcessFailedException | ||
| */ | ||
| public function onScriptPost() |
There was a problem hiding this comment.
The name onScriptPost seems unclear to me. I would prefer a fooEventHandler or fooHandler style name.
| { | ||
| // Ensure PHP_CodeSniffer is installed | ||
| if ($this->isPHPCodeSnifferInstalled() === false) { | ||
| return; |
There was a problem hiding this comment.
False language construct. It would be better to invert the check (!== false or === true) and move the statement below in to the if body. It might even be possible to collapse both if's into one.
| */ | ||
| private function loadInstalledPaths() | ||
| { | ||
| $phpcs = new Process($this->phpCodeSnifferBin . ' --config-show installed_paths'); |
There was a problem hiding this comment.
Instead of creating a new Process three times you might want to look into using the ProcessBuilder factory.
$this->builder = new ProcessBuilder();
$this->builder->setPrefix($this->phpCodeSnifferBin);
...
$this->builder
->setArguments(array('--list', '--file=archive.tar.gz'))
->getProcess()
->mustRun() /* or ->run()*/
…nged naming and visibility of some methods & variables.
|
|
||
| // This changes in case we do have installed_paths | ||
| if (count($this->installedPaths) !== 0) { | ||
| $arguments = ['--config-set', 'installed_paths', implode(',', $this->installedPaths)]; |
There was a problem hiding this comment.
Is there a specific reason for not using array_push ($argument, implode(',', $this->installedPaths)); instead of defining the entire array again?
Oh, wait. Nevermind.
There was a problem hiding this comment.
Only the middle element is the same...
Initial commit of a version which works by changing PHP_CodeSniffer's configuration