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

Implement CS checking based on the WP_CLI_CS ruleset #46

Merged
merged 13 commits into from
Apr 19, 2019
Merged

Implement CS checking based on the WP_CLI_CS ruleset #46

merged 13 commits into from
Apr 19, 2019

Conversation

thrijith
Copy link
Member

@thrijith thrijith commented Apr 17, 2019

Add a PHPCS ruleset using the new WPCliCSstandard.

Fixes #44

Related wp-cli/wp-cli#5179

@thrijith thrijith marked this pull request as ready for review April 17, 2019 19:08
@thrijith thrijith requested a review from a team as a code owner April 17, 2019 19:08
cron-command.php Outdated Show resolved Hide resolved
phpcs.xml.dist Outdated Show resolved Hide resolved
phpcs.xml.dist Outdated Show resolved Hide resolved
phpcs.xml.dist Outdated Show resolved Hide resolved
Update file exclude pattern in ruleset and minor fixes
Copy link

@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.

@thrijith Looking good! Thanks for all your efforts.

I've left some feedback in-line to contemplate. Not everything needs to be actioned, some things are just questions.

phpcs.xml.dist Outdated Show resolved Hide resolved
src/Cron_Command.php Outdated Show resolved Hide resolved
src/Cron_Command.php Outdated Show resolved Hide resolved
src/Cron_Event_Command.php Outdated Show resolved Hide resolved
src/Cron_Event_Command.php Show resolved Hide resolved
src/Cron_Event_Command.php Outdated Show resolved Hide resolved
src/Cron_Event_Command.php Outdated Show resolved Hide resolved
src/Cron_Event_Command.php Show resolved Hide resolved
src/Cron_Event_Command.php Show resolved Hide resolved
Update code usage where namespace function was used
Copy link

@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.

LGTM

Copy link

@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.

Just noticed one more thing...

),
);
$cron_request = apply_filters(
'cron_request', // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- 'cron_request', // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Calling native WordPress hook.
Copy link

Choose a reason for hiding this comment

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

Something looks wrong here, two comments instead of just the one.

Suggestion:

		// phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Calling native WordPress hook.
		$cron_request = apply_filters( 'cron_request', $cron_request_array );

@schlessera schlessera changed the title Implement CS checking based on the WPCliCS ruleset Implement CS checking based on the WP_CLI_CS ruleset Apr 19, 2019
src/Cron_Event_Command.php Outdated Show resolved Hide resolved
src/Cron_Event_Command.php Outdated Show resolved Hide resolved
@schlessera schlessera merged commit dfff746 into wp-cli:master Apr 19, 2019
@thrijith thrijith deleted the feature/use-phpcs branch April 19, 2019 17:05
danielbachhuber pushed a commit that referenced this pull request Nov 18, 2022
Implement CS checking based on the `WP_CLI_CS` ruleset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adopt and enforce new WP_CLI_CS standard
3 participants