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

Fix several issues and refactor code to conform WPCS #8

Closed
wants to merge 6 commits into from

Conversation

kagg-design
Copy link

Fix tests.
Fix check that Redis class exists.
Fix warning that occurs in some scenarios.
Refactor code to conform WPCS.

Now they run properly as for single site, so for multisite.

[FIXUP] Fix Travis with php 5.6.

[FIXUP] Fix Redis port.
[FIXUP] Fixed bug in the code.

[FIXUP] Fix WPCS in tests.

[FIXUP] Add package.
When several installs do not define WP_CACHE_KEY_SALT, they used same cache keys, what produced multiple bugs on sites.
@@ -463,7 +463,7 @@ public function __construct( $redis_instance = null ) {
* multi single WP installs on the same server.
*/
if ( ! defined( 'WP_CACHE_KEY_SALT' ) ) {
define( 'WP_CACHE_KEY_SALT', '' );
define( 'WP_CACHE_KEY_SALT', ABSPATH );

Choose a reason for hiding this comment

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

Discussed in #16

@@ -561,7 +561,7 @@ public function get( $_key, $group = 'default', $force = false ) {

if ( isset( $this->to_unserialize[ $redis_key ] ) ) {
unset( $this->to_unserialize[ $redis_key ] );
$value = unserialize( $value );
$value = maybe_unserialize( $value );

Choose a reason for hiding this comment

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

This can actually spawn more errors. Because unserialize returns an empty value and a warning that something is wrong, which is arguably safer and more predictable than what maybe_unserialize returns - the value itself which is unexpected.

Moved to #19

Copy link

@soulseekah soulseekah left a comment

Choose a reason for hiding this comment

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

This change is too large to go through. We can't see what's going on, the whole file has been changed.
Please resubmit as its own PR. Do not clump commits into one PR. Such PRs will be discarded.

@soulseekah
Copy link

Thank you for your contribution. We'll now close this PR as it greatly strays away from our contributing guidelines. We value your time.

Please see https://github.com/pressjitsu/pj-object-cache-red/blob/master/CONTRIBUTING.md

@soulseekah soulseekah closed this Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants