Skip to content

Conversation

@GaryJones
Copy link
Member

See the individual commits for more detailed information.

Makes it easier to see that PHPCS actually ran.
The phpcodesniffer-composer-installer plugin recognises composer packages with the:

`"type": "phpcodesniffer-standard"`

value, and will automatically register the standard with PHP_CodeSniffer.
WordPress Coding Standards now supports PHP_CodeSniffer v3 (3.0.2+), and has it as one of the required dependencies, so no need to limit PHPCS to the 2.9 branch.

`composer.lock` references PHPCS 3.0.2
This standard checks for PHP syntax that was only introduced in a version of PHP later than the desired minimum version.

Assuming PHP 5.2 is the minimum version, as per WordPress core, the only error was a `__dir__` magic constant in the `phpunit/bootstrap.php` file.
Four spaces is what Composer uses itself when updating `composer.json`, so for constistency, it makes to enforce this when editing it manually as well.
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to composer install from an existing installation resulted in an error for me:

[Symfony\Component\Process\Exception\ProcessFailedException]                                                                                                                                           
  The command "'/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/vendor/bin/phpcs' '--config-show' 'installed_paths'" failed.                                             
  Exit Code: 255(Unknown error)                                                                                                                                                                          
  Working directory: /Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg                                                                                                     
  Output:                                                                                                                                                                                                
  ================                                                                                                                                                                                       
  Warning: require(/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/vendor/composer/../symfony/polyfill-mbstring/bootstrap.php): failed to open stream: No such file or   
  directory in /Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/vendor/composer/autoload_real.php on line 66       

But after clearing out my vendor directory and trying again, it worked fine.

Looks good 👍

@aduth aduth merged commit 23f599c into WordPress:master Sep 1, 2017
aduth added a commit that referenced this pull request Sep 1, 2017
Wasn't caught 'til merge of #2611, since issue was introduced after original branch time
@ntwb
Copy link
Member

ntwb commented Sep 1, 2017

@GaryJones Why switch composer.json to spaces?

We explicitly changed it to use tabs in #1067 (comment)

Edit: I didn't read the commit message "Four spaces is what Composer uses itself when updating composer.json, so for consistency, it makes to enforce this when editing it manually as well.". We should be consistent with our coding standards and tabs usage where we can and that is to use tabs for indentation everywhere where possible including composer.json 😄

@GaryJones GaryJones deleted the composer-phpcs branch September 1, 2017 06:42
@aduth
Copy link
Member

aduth commented Sep 1, 2017

Having more experience with package.json which had done the same, it doesn't seem a worthwhile use of time to try to avoid the automated install tools and maintain the file's indentation manually.

We already allow the two-space in package.json:

gutenberg/.editorconfig

Lines 11 to 13 in a2756aa

[{package.json,*.yml}]
indent_style = space
indent_size = 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants