-
Notifications
You must be signed in to change notification settings - Fork 29
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
[CMSP-624] Check for and recommend Object Cache Pro #147
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Chris Reynolds <chris@jazzsequence.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the first if
got swallowed in applying the changes, so there's just a } else {
which will be a syntax error and break the command.
The Behat tests should be updated as well so they are passing before we merge this. Let us know if you need us to dig into those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple more tweaks to the logic.
- Check if
WP_REDIS_OBJECT_CACHE
is defined. This tells us if WP Redis is installed. If it is, give them a message that says "Hey, you should use OCP instead." Otherwise... - Check if
WP_REDIS_CONFIG
is defined. In this case we are checking to see if it doesn't exist, because we want to tell them to install it if they don't have it. If it's already defined... - Return success, OCP is found.
I wasn't able to do it in my snippet without breaking things, but with the new elseif
it should probably be on the same line as the closing }
from the previous if
.
Check for and recommend Object Cache Pro