roots / trellis

WordPress LEMP stack with PHP 8.2, Composer, WP-CLI and more
https://roots.io/trellis/
MIT License
2.51k stars 607 forks source link

Remove `WPMU_PLUGIN_DIR` constant to `null` in `tmp_multisite_constants.php` #1386

Closed strarsis closed 2 years ago

strarsis commented 2 years ago

(Variant A) This PR removes the line that sets the WPMU_PLUGIN_DIR constant to null in tmp_multisite_constants.php so the Ansible check that uses wp core is-installed [...] passes (failing since the WordPress 6.0 release).

Before merging, it may be good to think about possible issues that may arise with not setting the WPMU_PLUGIN_DIR constant to null in that (smoke) test during deployment. Can this result in passing deploys for multisite setups that would have issues, hence should not have passed this check? Previous discussions (https://github.com/roots/trellis/issues/1385) indicate that this shouldn't be the case and that setting that constant to null isn't really required anymore with recent WordPress versions.

Fixes issue: https://github.com/roots/trellis/issues/1385

swalkinshaw commented 2 years ago

If someone else could help test with a multisite setup that would be helpful. I'm on vacation but I'm happy to get this merged; I just can't really help test.

I'll give this ~24 hours anyway since non-multisite setups are way more common so it's better to fix those at least.

strarsis commented 2 years ago

@swalkinshaw: An alternative PR is also in preparation that will let the Ansible check just ignore that particular PHP warning. This would be an alternative if setting that WPMU_PLUGIN_DIR constant to null somehow proves to be important for multisite. And the check could be split into two different checks, one for non-multisite and one specifically for multisites.

strarsis commented 2 years ago

@swalkinshaw: Alternative PR that makes the WordPress installed? check ignore this particular PHP warning: https://github.com/roots/trellis/pull/1387

Variant A PR: (This PR) Variant B PR: https://github.com/roots/trellis/pull/1387 Variant C PR: https://github.com/roots/trellis/pull/1388

strarsis commented 2 years ago

Closed as Variant C PR (https://github.com/roots/trellis/pull/1388) was merged.