Open
Conversation
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #94.
This PR adds support for PHP 8.x versions by bumping dependency versions, and making some changes to tests so that they run under PHP 8.5 (latest stable).
Testing procedure
Tested with every minor 8.x version (8.0, 8.1, 8.2, 8.3, 8.4, and 8.5). Development and testing was done on 8.5.
Commands run on every version:
composer validate --strictcomposer installwp valet new <name>Commands run on 8.5:
composer behatTests pass:
PHP 8.5 notes
Because 8.5 is so recent, I needed to use the nightly build of WP-CLI to avoid (harmless) noise:
wp cli update --nightly(version:WP-CLI 2.13.0-alpha-b0f2aec).Additionally, one warning shown by
composer install(andupdate):roots/wp-password-bcryptis abandoned nowbcryptis in WP core as of 6.8. We need to just ignore that for the time being.Changes made to tests
I tried to run the tests under every minor PHP 8.x version but couldn't due to dependency issues. PHP 8.0 and 8.1 (both now EOL) couldn't run it at all, but I made sure
wp valet new <name>works on every 8.x version.For the Bedrock tests,
$_SERVERenv vars weren't returning and those tests failed. I fixed that by usinggetenv()instead.Due to being unfamiliar with SQLite, I got stuck here:
I changed "run" to "try" to get it to pass, but it's... imperfect(!).
I see you made
aaemnnosttv/wp-sqlite-db, which usessqliteCreateFunction. I am happy to make a PR there to patch that, which means we can revert to "run" here and also require the new sqlite db package here. I'm happy to do it if you're okay with it? :)Finally, I changed from a hardcoded
mysqlinfeatures/Context/FeatureContext.phpto aMYSQL_BINARYenv var instead. My dev setup uses mariadb and the tests intermittently failed otherwise. This PR onwp-cli/wp-cli-testswas useful.Major version bump?
Since these changes expressly require PHP 8.x or newer (
composer require php >=8.0), I was wondering if you think this release should be tagged version 2.x, or similar? Let me know if you want me to merge into a different branch accordingly.Thanks so much. 😄