-
Notifications
You must be signed in to change notification settings - Fork 46
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
Ensure blog is installed before creating tables. #82
Ensure blog is installed before creating tables. #82
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.
Linting failed (19 errors, 38 warnings).
(56 notices occurred in your codebase, but were on files/lines not included in this PR.)
If the site is not installed, cancel bootstrapping on `plugins_loaded` and then rebootstrap on the `wp_install` action. Ensure site meta is populated when installing a network via the WP CLI script by adding the data to the `populate_network_meta` filter.
Hmm @roborourke @rmccue did we just fix something similar to this? Seem to remember needing to patch cavalcade when WP_INSTALLING was set... something along those lines? |
The issue is more to do with the runner, it doesn’t really handle a pre installation state and generates errors or quits the service entirely |
Ok thanks @roborourke, @rmccue can you review and merge accordingly? |
@peterwilsoncc can you update per above? (I can't commit to your fork :) ) |
Co-Authored-By: Ryan McCue <me@ryanmccue.info>
Co-Authored-By: Ryan McCue <me@ryanmccue.info>
Co-Authored-By: Ryan McCue <me@ryanmccue.info>
Requested changes committed along with updates from |
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.
Thanks @peterwilsoncc :)
@roborourke @rmccue Bumped with master again to include the CI fix. Tests are now passing. It'll need some confirmation that the changes to plugin.php and namespace.php don't screw anything but testing locally it looks good to me. |
# Conflicts: # plugin.php
7678307
to
9781c10
Compare
@roborourke Can you approve and merge this PR in for @peterwilsoncc |
@stuartshields yeah, been meaning to test it locally. I was hoping to get #91 in first because the bulk of the tests are actually skipped right now |
…alcade into 79-ensure-wp-installed
WP fires the
plugins_loaded
hook during the install process, causing the Cavalcade tables to be created before the options table exists.Without an options table, Cavalcade is unable to store the version of the database it is using it ends up considering all sites to be using version 1 of the database schema.
This adds a check to ensure
is_blog_installed() == true
before adding the Cavalcade database tables. I'm not sure how this affects multisite so would value any feedback.Fixes #73