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

add --show-grant argument to wp cap list and --grant to wp cap add #19

Merged
merged 5 commits into from Apr 19, 2018

Conversation

2 participants
@kshaner
Copy link
Contributor

kshaner commented Mar 8, 2018

Addresses #18

@schlessera schlessera added this to the 1.0.6 milestone Apr 10, 2018

@schlessera
Copy link
Member

schlessera left a comment

Thanks for the PR, @kshaner!
The code looks good, I just requested changes for a lot of nitpicky things like code style and comments.

@@ -45,6 +45,12 @@ class Capabilities_Command extends WP_CLI_Command {
* - count
* - yaml
* ---
*
* [--show-grant]
* : Display all capabilites defined for role including grant

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

Typo: capabilites => capabilities

@@ -60,26 +66,43 @@ class Capabilities_Command extends WP_CLI_Command {
*/
public function list_( $args, $assoc_args ) {
$role_obj = self::get_role( $args[0] );
$show_grant = !empty( $assoc_args['show-grant'] );

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

Code style: space after !

$formatter = new \WP_CLI\Formatter( $assoc_args, $this->fields );
$formatter->display_items( $output_caps );
}
}

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

Code style: Unneeded indentation

self::persistence_check();

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

Code style: Unneeded indentation

@@ -146,7 +187,7 @@ public function remove( $args ) {
$count = 0;
foreach ( $args as $cap ) {
if ( !$role_obj->has_cap( $cap ) )
if ( !isset( $role_obj->capabilities[ $cap ] ) )

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

Code style: space after !

$role = array_shift( $args );
$role_obj = self::get_role( $role );
$grant = !isset( $assoc_args['grant'] ) || !empty( $assoc_args['grant'] );

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

Code style: space after !

@@ -45,6 +45,12 @@ class Capabilities_Command extends WP_CLI_Command {
* - count
* - yaml
* ---
*
* [--show-grant]
* : Display all capabilites defined for role including grant

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

Code Style: Comment should end with a period: .

@@ -90,32 +113,50 @@ public function list_( $args, $assoc_args ) {
*
* <cap>...
* : One or more capabilities to add.
*
* [--grant]
* : Add the capability as true/false

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

Code Style: Comment should end with a period: .

This comment has been minimized.

@schlessera

schlessera Apr 10, 2018

Member

I'd reword this to make it clearer.
How about:

Adds the capability as an explicit boolean value, instead of implicitly defaulting to `true`.
@kshaner

This comment has been minimized.

Copy link
Contributor Author

kshaner commented Apr 11, 2018

Thanks @schlessera. I've addressed the spacing and agree with your updated comment. It looks like the travis-ci build ran out of memory on the check though. Not sure how I can address that.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Apr 19, 2018

@kshaner Thanks for the good work on this.

I've fixed the memory issue for the Travis 5.3 branch. If you merge the latest changes from master, the tests should pass.

@kshaner

This comment has been minimized.

Copy link
Contributor Author

kshaner commented Apr 19, 2018

@schlessera, Thanks for fixing the php memory limit. All checks pass now!

@schlessera schlessera merged commit 334ccef into wp-cli:master Apr 19, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.