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

First pass at extending the 'delete' command to support deleting arbitra... #165

Merged
merged 6 commits into from Sep 22, 2012

Conversation

2 participants
@danielbachhuber
Member

danielbachhuber commented Sep 15, 2012

...ry sets of posts based on standard WP_Query arguments

See #160

First pass at extending the 'delete' command to support deleting arbi…
…trary sets of posts based on standard WP_Query arguments
@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Sep 15, 2012

Member

Happy to hear any and all feedback on coding style, etc. It might be nice to add a few more arguments like 'start_date' and 'end_date', and to abstract the argument validation out a bit.

Member

danielbachhuber commented Sep 15, 2012

Happy to hear any and all feedback on coding style, etc. It might be nice to add a few more arguments like 'start_date' and 'end_date', and to abstract the argument validation out a bit.

@scribu

View changes

Show outdated Hide outdated src/php/wp-cli/commands/internals/post.php Outdated
@scribu

View changes

Show outdated Hide outdated src/php/wp-cli/commands/internals/post.php Outdated
@scribu

This comment has been minimized.

Show comment
Hide comment
@scribu

scribu Sep 15, 2012

Member

Would be nice to have src/docs/post-delete.txt updated too.

Member

scribu commented Sep 15, 2012

Would be nice to have src/docs/post-delete.txt updated too.

foreach( $posts_to_delete as $post_id ) {
if ( wp_delete_post( $post_id, (bool)$assoc_args['force'] ) ) {
$action = ( (bool) $assoc_args['force'] ) ? 'Deleted' : 'Trashed';
WP_CLI::success( "{$action} post $post_id." );

This comment has been minimized.

@scribu

scribu Sep 15, 2012

Member

I think printing a message for every single post is too verbose, especially since $action is loop-invariant.

Could print a single message: Deleted X posts.

@scribu

scribu Sep 15, 2012

Member

I think printing a message for every single post is too verbose, especially since $action is loop-invariant.

Could print a single message: Deleted X posts.

This comment has been minimized.

@danielbachhuber

danielbachhuber Sep 15, 2012

Member

I prefer the verbose output, as you have post_ids in your logs to check against if you accidentally typed in the wrong parameters, etc. The final count would be useful though

@danielbachhuber

danielbachhuber Sep 15, 2012

Member

I prefer the verbose output, as you have post_ids in your logs to check against if you accidentally typed in the wrong parameters, etc. The final count would be useful though

This comment has been minimized.

@scribu

scribu Sep 15, 2012

Member

That's a fair point. Don't really need the count then, since you can just pipe it to wc -l.

@scribu

scribu Sep 15, 2012

Member

That's a fair point. Don't really need the count then, since you can just pipe it to wc -l.

This comment has been minimized.

@scribu

scribu Sep 15, 2012

Member

But $action is still loop-invariant and therefore should be computed only once, before the loop.

Ideally, though, you'd check if it was actually deleted or just trashed. If it involves doing an additional query per post, then forget it. :)

@scribu

scribu Sep 15, 2012

Member

But $action is still loop-invariant and therefore should be computed only once, before the loop.

Ideally, though, you'd check if it was actually deleted or just trashed. If it involves doing an additional query per post, then forget it. :)

This comment has been minimized.

@danielbachhuber

danielbachhuber Sep 15, 2012

Member

True. I'll take another look when I'm back at the computer.

@danielbachhuber

danielbachhuber Sep 15, 2012

Member

True. I'll take another look when I'm back at the computer.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Sep 15, 2012

Member

Updated docs in 7640b7e

Member

danielbachhuber commented Sep 15, 2012

Updated docs in 7640b7e

@scribu

This comment has been minimized.

Show comment
Hide comment
@scribu

scribu Sep 15, 2012

Member

Ugh... "This pull request cannot be automatically merged."

Before we continue, please either rebase the branch to the latest HEAD or merge the latest HEAD and fix the conflicts.

Member

scribu commented Sep 15, 2012

Ugh... "This pull request cannot be automatically merged."

Before we continue, please either rebase the branch to the latest HEAD or merge the latest HEAD and fix the conflicts.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Sep 15, 2012

Member

Weird... I just branched 20 minutes ago :/ I'll see what I can do.

Member

danielbachhuber commented Sep 15, 2012

Weird... I just branched 20 minutes ago :/ I'll see what I can do.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Sep 15, 2012

Member

If that's too ugly, I can create a new branch from master and cherry pick my commits to it.

Member

danielbachhuber commented Sep 15, 2012

If that's too ugly, I can create a new branch from master and cherry pick my commits to it.

@scribu

This comment has been minimized.

Show comment
Hide comment
@scribu

scribu Sep 15, 2012

Member

It's fine; thanks.

Member

scribu commented Sep 15, 2012

It's fine; thanks.

@@ -18,3 +30,5 @@ wp-post-delete(1) -- Delete a WordPress post.
## EXAMPLES
wp post delete 123 --force
wp post delete --post_type=page --post_status=draft

This comment has been minimized.

@scribu

scribu Sep 15, 2012

Member

Wondering if we should strip the 'post_' prefix from parameters. It's currently inconsistent between commands:

wp post create --post_type=foo --post_status=draft

vs.

wp generate posts --type=foo --status=draft
@scribu

scribu Sep 15, 2012

Member

Wondering if we should strip the 'post_' prefix from parameters. It's currently inconsistent between commands:

wp post create --post_type=foo --post_status=draft

vs.

wp generate posts --type=foo --status=draft

This comment has been minimized.

@danielbachhuber

danielbachhuber Sep 15, 2012

Member

I generally prefer the former because it's consistent with core and hopefully means you don't have to guess what the parameters are when switching between commands (although currently I suppose you do need to guess)

@danielbachhuber

danielbachhuber Sep 15, 2012

Member

I generally prefer the former because it's consistent with core and hopefully means you don't have to guess what the parameters are when switching between commands (although currently I suppose you do need to guess)

This comment has been minimized.

@scribu

scribu Sep 15, 2012

Member

Yeah; I think I'll switch wp generate posts over in the future.

@scribu

scribu Sep 15, 2012

Member

Yeah; I think I'll switch wp generate posts over in the future.

@scribu scribu merged commit 52f99a3 into wp-cli:master Sep 22, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment