-
-
Notifications
You must be signed in to change notification settings - Fork 933
Add support for PHP 8 #3745
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
Add support for PHP 8 #3745
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.
This looks good! I fear it introduces some small BC breaks but I don't think that they can be prevented.
tests/Bridge/Symfony/Bundle/DataCollector/RequestDataCollectorTest.php
Outdated
Show resolved
Hide resolved
What's the status of this PR? I think the failing test is unrelated, isn't it? |
Theoretically, this PR should be rebased against 2.5 as we consider supporting supporting new PHP versions as a bug fixes. However this PR introduces small (unavoidable) BC breaks, so merging it in a patch release doesn't look reasonable. WDYT @api-platform/core-team? |
I think having this support for 2.6 is enough. |
👍 for 2.6 and the BC breaks must be added in the release note/changelog (+ maybe a new UPGRADING.md ?) |
The problem is that we still have no ETA for 2.6. If only this version supports PHP 8, we'll have to release 2.6 before or just after the release of PHP 8. |
The release of PHP 8 is the 26th of November. It's a good ETA don't you think? |
Many things aren't ready yet. Especially regarding Vulcain's support in JS components. |
Can't it be done later? |
I'm not sure yet. Merging in 2.5 would be easier. |
@dunglas Which changes are the BC breaks? Maybe there is a way around it, but I'm not sure which changes will break BC currently. |
tests/Bridge/Symfony/Bundle/DataPersister/TraceableChainDataPersisterTest.php
Show resolved
Hide resolved
@pierredup ok I double checked and you're right, we're (again) safe because most of our classes are final 😬! Thanks! |
doctrine/dbal released new version 2.12.x which supports php8, no bc/code updates needed only update doctrine/dbal to 2.12.x. So is there any ETA for php8? Also u need to update https://github.com/willdurand/Negotiation package which supports php8 only in mater branch (3.0-dev) for now. |
@dotdevru we'll merge this PR as soon as possible. |
@pierredup do you want me to rebase it against 2.5? |
@dunglas I've rebased on 2.5 and removed the skipped tests since Doctrine DBAL now supports PHP 8. |
Any estimate when this will be merged (and tagged)? |
Thank you very much @pierredup! Great work. @stephanvierkant we need to finish #3794 before tagging a release. |
Add support for PHP 8.
This PR removes some deprecations (mostly defining a required argument after an optional argument). It also aliases the
Match
class from Doctrine Mongodb sincematch
is now a reserved keyword in PHP 8 and causes parse errors.All the tests pass if they are run individually. The biggest outstanding issue is with Doctrine. Currently, it fails with the error
Fatal error: Declaration of Doctrine\DBAL\Driver\PDOConnection::query() must be compatible with PDO::query(string $query, ?int $fetch_mode = null, mixed ...$fetch_mode_args)
.This specific issue has been fixed in Doctrine 3.x (currently unreleased), but it's not possible to add it as a dev dependency currently as there are still a lot of other dependencies that don't allow Doctrine 3.x yet.