Add wp cache flush-group for flushing a cache group
#85
Conversation
Needs extra input for testing.
Also phpcs formatting.
src/Cache_Command.php
Outdated
| * # Clear cache group. | ||
| * $ wp cache clear-group my_group | ||
| * | ||
| * @subcommand clear-group |
There was a problem hiding this comment.
@spacedmonkey Should it be wp cache clear-group, wp cache delete-group, or wp cache flush-group?
There was a problem hiding this comment.
@tillkruss Maybe you might have an opinion on naming ^ ?
There was a problem hiding this comment.
I'd stick with wp cache flush-group to be identical to wp_cache_flush_group(), but I'm a fan of @alias wp cache clear-group.
There was a problem hiding this comment.
Hm... I think it should've been named wp_cache_delete_group() in core 😁
flush-group is too close to flush, in my opinion. But, I guess we should be consistent with naming.
@wp-cli/committers Any other opinions to bring into the mix?
There was a problem hiding this comment.
Yeah, alias delete and clear makes sense.
There was a problem hiding this comment.
Having worked on it a bit more I think I would prefer flush-group it might be close to the existing flush but I think its more consistent that way.
There was a problem hiding this comment.
Having worked on it a bit more I think I would prefer
flush-groupit might be close to the existingflushbut I think its more consistent that way.
Sounds good to me 👍
features/cache.feature
Outdated
| Then STDERR should be: | ||
| """ | ||
| Warning: Cache group 'false_return' was not be flushed. | ||
| """ |
There was a problem hiding this comment.
I don't know how to create/fake a cache implementation for better behat testing.
@spacedmonkey Are there any cache implementations you're aware of that we could easily install?
There was a problem hiding this comment.
You could use this as upstream: https://github.com/rhubarbgroup/redis-cache/blob/develop/includes/object-cache.php
It will always have the latest Redis implementation ahead of releases.
There was a problem hiding this comment.
👍
I might try to find a SQLite implementation so we don't have to add Redis to CI.
There was a problem hiding this comment.
There was a problem hiding this comment.
Thank's I'll take a look tomorrow or monday.
There was a problem hiding this comment.
https://github.com/OllieJones/sqlite-object-cache does not like WP-CLI....
But I added a mu-plugin that overrides the $GLOBALS['wp_object_cache'].
Ideally a drop-in would be created but that would be bigger then the whole feature test file.
This also works.
danielbachhuber
changed the title
wp cache clear-group for flushing a cache group
|
@danielbachhuber the failed tests are valid, the errors should trigger with older WP versions. |
|
@janw-me Yes, please:
|
|
@danielbachhuber Where I do personally agree, I think delete group would be better. But there are plugins that used that already so we could not use it. Let's use flush-group, as this is the naming in core, even through it sucks, it sucks in the WordPress way. |
Fixed failing test and generated readme file.
| Then STDOUT should be: | ||
| """ | ||
| Success: Cache group 'add_multiple' was flushed. | ||
| """ |
There was a problem hiding this comment.
You'll want to move this step to a new scenario so you can control it with @require-wp-6.1
danielbachhuber
changed the title
wp cache clear-group for flushing a cache groupwp cache flush-group for flushing a cache group
Needs extra input for testing.
I don't know how to create/fake a cache implementation for better behat testing.
Fixes #80