Skip to content

Conversation

@wojsmol
Copy link
Contributor

@wojsmol wojsmol commented Apr 20, 2019

Add a PHPCS ruleset using the new WP_CLI_CS standard.

Fixes #34

Related wp-cli/wp-cli#5179

@wojsmol wojsmol requested a review from a team as a code owner April 20, 2019 15:02
</property>
</properties>
</rule>
<!-- Exclude existing classes from the prefix rule as it would break BC to prefix them now. -->
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a line break here for better readability.

Suggested change
<!-- Exclude existing classes from the prefix rule as it would break BC to prefix them now. -->
<!-- Exclude existing classes from the prefix rule as it would break BC to prefix them now. -->

phpcs.xml.dist Outdated
<rule ref="WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedClassFound">
<exclude-pattern>*/src/Super_Admin_Command\.php$</exclude-pattern>
</rule>
</ruleset> No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

All texts files should end with a line break.

Suggested change
</ruleset>
</ruleset>


public function __construct() {
$this->fetcher = new \WP_CLI\Fetchers\User;
$this->fetcher = new \WP_CLI\Fetchers\User();
Copy link
Member

Choose a reason for hiding this comment

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

While we're make CS changes, let's import classes like these so we can see the dependencies at the top of the file and have clean class names in the code.

As the pure class name here is not very telling without the preceding namespace (User), I'd add an alias to the import at the top:

use WP_CLI\Fetchers\User as UserFetcher;

Then, the code becomes clear and nice here:

Suggested change
$this->fetcher = new \WP_CLI\Fetchers\User();
$this->fetcher = new UserFetcher();

* @subcommand list
*/
public function _list( $_, $assoc_args ) {
public function list_( $_, $assoc_args ) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd go for something more meaningful here, as ending in an underscore is still "weird" in terms of method name.

Suggested change
public function list_( $_, $assoc_args ) {
public function list_subcommand( $_, $assoc_args ) {

@schlessera schlessera added this to the 2.0.2 milestone Apr 20, 2019
@schlessera schlessera merged commit bd1543c into wp-cli:master Apr 20, 2019
@wojsmol wojsmol deleted the use-wp_cli_cs branch April 20, 2019 23:32
// Exclude numeric and email-like logins (login names can be email-like but ignore this given the circumstances).
return ! isset( $flipped_user_logins[ $v ] ) && ! is_numeric( $v ) && ! is_email( $v );
} ) ) );
$user_logins = array_merge(
Copy link

Choose a reason for hiding this comment

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

For a future iteration, I'd like to suggest splitting these nested function calls to separate lines to improve readability.

			$filtered_args     = array_filter(
				$args,
				function ( $v ) use ( $flipped_user_logins ) {
					// Exclude numeric and email-like logins (login names can be email-like but ignore this given the circumstances).
					return ! isset( $flipped_user_logins[ $v ] ) && ! is_numeric( $v ) && ! is_email( $v );
				}
			);
			$filtered_args     = array_unique( $filtered_args );
			$user_logins       = array_merge( $user_logins, $filtered_args );

schlessera added a commit that referenced this pull request Jan 5, 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