Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use phpcs.xml.dist instead of custom-named phpcs.ruleset.xml #19

Merged
merged 4 commits into from Jun 5, 2017

Conversation

2 participants
@westonruter
Copy link
Contributor

commented Jun 3, 2017

Incorporating improvements from xwp/wp-dev-lib#243:

The phpcs.ruleset.xml was a custom filename used to disambiguate from the a generic ruleset.xml which used to be what PHPCS looked for until squizlabs/PHP_CodeSniffer@ae995b5 switched this to phpcs.xml and then squizlabs/PHP_CodeSniffer@85cbd46 added support for phpcs.xml.dist as the next-level fallback, bringing it in alignment in behavior with PHPUnit's phpunit.xml and phpunit.xml.dist.

When the standard filenames are used, they do not need to be supplied on the command line and will be automatically discovered by PHPCS by traversing the directory tree, just as ESLint and any number of other tools do.

Additionally, in the past PHPCS-lib would often need to get invoked like:

phpcs -s --extensions=php .

These arguments are now unnecessary and are supplied by the default ruleset:

  • -s: Show sniff codes in all reports
  • --extensions=php: Limit reporting to PHP files.
  • .: By indicating a default file, PHPCS won't assume to look at STDIN, allowing you to just invoke phpcs instead of phpcs ..

First realized in #2 (comment)
The phpcs.ruleset.xml filename originally came from xwp/wp-dev-lib@9d1d025
PR which used filename in WP-CLI: wp-cli/wp-cli#3472
See also xwp/wp-dev-lib#231
Also see WordPress/WordPress-Coding-Standards#932

PhpStorm should soon support automatically looking for phpcs.xml(.dist) as it does for ESLint and other tools' configs. See https://youtrack.jetbrains.com/issue/WI-36028

westonruter added some commits Jun 3, 2017

Remove now-reundant --standard arg for phpcs
Also remove redundant --configuration param from call to phpunit
Use default phpcs command args in project ruleset
Eliminates need to pass list of files to check and/or '--extensions=php .'
Include -s arg by default when invoking PHPCS
This ensures that the sniff codes are shown in all reports

@westonruter westonruter force-pushed the xwp:update/phpcs branch from 10b401d to a5bb586 Jun 3, 2017

- phpcs --standard=phpcs.xml.dist $(find . -name '*.php')

This comment has been minimized.

Copy link
@schlessera

schlessera Jun 4, 2017

Member

I think that the --standard=... should be removed in the CI configurations as well.

Otherwise, you might end up creating a customized rule set that is being used for local tests, while the CI still runs with the default tests.

This comment has been minimized.

Copy link
@westonruter

westonruter Jun 4, 2017

Author Contributor

Isn't this what was done in c1fd60d?

This comment has been minimized.

Copy link
@schlessera

schlessera Jun 5, 2017

Member

Oh, my bad, I thought I was looking at the merged diff.

@@ -56,5 +56,5 @@ script:
fi
- |
if [[ "$WP_TRAVISCI" == "phpcs" ]] ; then
phpcs --standard=phpcs.ruleset.xml $(find . -name '*.php')
phpcs --standard=phpcs.xml.dist $(find . -name '*.php')

This comment has been minimized.

Copy link
@schlessera

schlessera Jun 4, 2017

Member

Same as with the GitLab file above.

This comment has been minimized.

Copy link
@westonruter

westonruter Jun 4, 2017

Author Contributor

See c1fd60d?

@schlessera schlessera merged commit fd3a648 into wp-cli:master Jun 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2017

FYI: I also blogged about the background for this change in https://make.xwp.co/2017/06/04/reducing-command-line-args-required-for-running-phpcs/

@westonruter westonruter deleted the xwp:update/phpcs branch Jun 5, 2017

@schlessera schlessera added this to the 1.0.5 milestone Jun 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.