Skip to content

Implement CS checking based on the WP_CLI_CS ruleset#117

Merged
schlessera merged 35 commits intowp-cli:masterfrom
wojsmol:use-wp-cli-cs
Apr 24, 2019
Merged

Implement CS checking based on the WP_CLI_CS ruleset#117
schlessera merged 35 commits intowp-cli:masterfrom
wojsmol:use-wp-cli-cs

Conversation

@wojsmol
Copy link
Copy Markdown
Contributor

@wojsmol wojsmol commented Apr 21, 2019

Add a PHPCS ruleset using the new WP_CLI_CS standard.

Fixes #110

Related wp-cli/wp-cli#5179

@wojsmol wojsmol requested a review from a team as a code owner April 21, 2019 10:32
Copy link
Copy Markdown
Contributor

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@wojsmol Thanks for this PR!

I've left a number of comments in-line. Quite a few are just suggestions, but this PR - as is - also introduces a few bugs due to strict comparisons being introduced incorrectly. Please be very careful with those. My remarks will hopefully help you fix them.

The 1.6 endpoint requires the unsafe `unserialize` method that is flagged by PHPCS.

By bumping the version to 1.7, we can replace that with JSON instead.
@schlessera
Copy link
Copy Markdown
Member

@wojsmol This package was more complicated, so I made a series of general CS tweaks still to get this command more in line with the rest of the packages.

I also added a few refactors to get rid of the remaining PHPCS issues.

Can you please review the changes I made?

@schlessera
Copy link
Copy Markdown
Member

We seem to be having the same issue for trunk here than with the language command:

 PHP Strict Standards:  Declaration of WP_CLI\Core\CoreUpgrader::download_package() should be compatible with WP_Upgrader::download_package($package, $check_signatures = false) in src/WP_CLI/Core/CoreUpgrader.php on line 162

@schlessera
Copy link
Copy Markdown
Member

I'll merge the PR as-is, the remaining failure is unrelated.

@schlessera schlessera added the command:core Related to 'core' command label Apr 24, 2019
@schlessera schlessera added this to the 2.0.4 milestone Apr 24, 2019
@schlessera schlessera merged commit 1dc9699 into wp-cli:master Apr 24, 2019
@wojsmol wojsmol deleted the use-wp-cli-cs branch April 24, 2019 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:core Related to 'core' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adopt and enforce new WP_CLI_CS standard

3 participants