-
Notifications
You must be signed in to change notification settings - Fork 5
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
wp config simplifed to avoid complications on non live envs #17
base: main
Are you sure you want to change the base?
wp config simplifed to avoid complications on non live envs #17
Conversation
pantheon.php
Outdated
@@ -45,6 +42,9 @@ | |||
if ( getenv( 'FRAMEWORK' ) === 'wordpress_network' && ! defined( 'WP_ALLOW_MULTISITE' ) ) { | |||
define( 'WP_ALLOW_MULTISITE', true ); | |||
} | |||
if ( defined( 'MULTISITE' ) && defined( 'WP_ALLOW_MULTISITE' ) && WP_ALLOW_MULTISITE ) { | |||
require_once 'inc/pantheon-network-setup.php'; | |||
} |
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.
These lines are unrelated and merged in #21. Can you remove these from the PR?
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 for the review. merged with the latest branch
inc/network/includes-network.php
Outdated
define( 'MULTISITE', true ); | ||
define( 'SUBDOMAIN_INSTALL', <?php echo $subdomain_install ? 'true' : 'false'; ?> ); | ||
define( 'DOMAIN_CURRENT_SITE', $hostname ); | ||
define( 'DOMAIN_CURRENT_SITE', $_SERVER['HTTP_HOST'] ); |
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.
Have you tested with Lando to confirm dropping the hard-coded hostname doesn't break anything locally?
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.
Tested with Lando and found issues:
- Getting a PHP Notice when running
lando wp
cli commands:PHP Notice: Undefined index: HTTP_HOST in phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1296) : eval()'d code on line 90
- When pulling a WPMS site, users need to do a
lando wp search-replace
with--all-tables
flag to include thewp_blogs
&wp_site
entries to be updated
Updated my PR to to fix the 1
For the 2 error, we might need to add a more verbose error to the generic error Error establishing a database connection
to make sure that all the URLs are replaced accordingly
fixes #16