-
Notifications
You must be signed in to change notification settings - Fork 24
Implement CS checking based on the WP_CLI_CS ruleset
#80
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
Conversation
... as that version contains the new WPCliCS PHPCS standard.
* `.distignore`: no need to distribute the phpcs/phpunit config files.
* `.gitignore`: Allow devs to overwrite config files for PHPUnit and PHPCS
- The `.dist` files should be committed. However, for their personal use, devs can overrule those files with versions without `.dist`.
Those personal versions should never be committed.
As a side-note: for PHPCS, having a personal version while still using the original `.dist` file is made very easy, as you can just import the `.dist` file as a starting point, like so:
```xml
<?xml version="1.0"?>
<ruleset name="WP-CLI">
<rule ref="./phpcs.xml.dist"/>
</ruleset>
```
Yes, it can be renamed or put into a proper namespace. However, that will make it inconsistent with the rest of the package, so I still think a whitelist is preferable here. |
That's what we get back from a remote call to the wordpress.org API. I don't think we can change these. |
Same here again. Technically, we can indeed rename this, but it just make the package as a whole more inconsistent. |
The file is completely documented in-line.
ef7cfe1 to
7433d7c
Compare
|
PR has been updated to reflect the above. I expect the build to pass now. |
|
Hmm.. the build is failing hard, but I somehow don't think the failure is related to this PR. The failures are to do with the |
|
No, it's unrelated to this PR. My current guess is that the |
|
Test failure unreleated to this PR and isolated to one single WP version. Merging anyway. |
* Composer: use wp-cli-tests ^2.1
... as that version contains the new WPCliCS PHPCS standard.
* .gitignore/.distignore: ignore phpcs/phpunit config files
* `.distignore`: no need to distribute the phpcs/phpunit config files.
* `.gitignore`: Allow devs to overwrite config files for PHPUnit and PHPCS
- The `.dist` files should be committed. However, for their personal use, devs can overrule those files with versions without `.dist`.
Those personal versions should never be committed.
As a side-note: for PHPCS, having a personal version while still using the original `.dist` file is made very easy, as you can just import the `.dist` file as a starting point, like so:
```xml
<?xml version="1.0"?>
<ruleset name="WP-CLI">
<rule ref="./phpcs.xml.dist"/>
</ruleset>
```
* PHPCS: add a ruleset for this project
The file is completely documented in-line.
* PHPCS: whitelist some code for select sniffs
This adds a PHPCS ruleset using the new WP_CLI_CS standard and adds some select whitelist comments.
Please see the individual commits for more detail.
Fixes #73
Related to wp-cli/wp-cli#5179
There are issues remaining for which I would like a second opinion on how to fix these:
☝️ As this is not a command, I'm wondering if the class can be renamed.
☝️ Regarding the object properties - these appear to be
stdClassobjects (based on visual code inspection only). If that's correct, I'm wondering if we can't just fix those rather than whitelist, though I'm open to whitelisting these.☝️ Here and in the previous file - these files are not the commands, so I'm wondering if we can fix the namespace rather whitelist it.