Skip to content
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

Provide initial compatibility with PHP 8 #2248

Merged
merged 8 commits into from
Dec 28, 2020

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Nov 26, 2020

Q A
Type improvement
BC Break no
Fixed issues Fixes #2222.

Summary

This PR makes the code base compatible with PHP 8. A few caveats:

  • The match functionality in aggregation builder requires changing as match is now a reserved keyword. This PR includes changes from Rename Match classes for PHP 8 compatibility #2237.
  • Tests are run on PHP 8, but we need to remove a few dev depdendencies as they block installing a PHP 8 compatible PHPUnit version.
  • Until ocramius/proxy-manager is compatible with PHP 8 (see Allow PHP 8 Ocramius/ProxyManager#628) we need to run composer install on PHP 8 with --ignore-platform-req=php.
  • Proxies and custom collections are not yet fully compatible with PHP 8 features such as union types. The former will be handled by ProxyManager but requires tests, while the latter will have to be taken care of in a separate pull request.

@alcaeus alcaeus added the Task label Nov 26, 2020
@alcaeus alcaeus added this to the 2.2.0 milestone Nov 26, 2020
@alcaeus alcaeus requested a review from malarzm November 26, 2020 10:28
@alcaeus alcaeus self-assigned this Nov 26, 2020
@jcaillot
Copy link

Thank you!

@alcaeus
Copy link
Member Author

alcaeus commented Nov 26, 2020

Note: I'll hold off on merging this until I've finished migrating the builds to GitHub actions. Can't rely on travis-CI anymore.

@garak
Copy link
Contributor

garak commented Dec 4, 2020

Any news on this? I see that CI is running on github actions already

@alcaeus
Copy link
Member Author

alcaeus commented Dec 4, 2020

I lost track of this but will revisit soon. Thanks for the reminder 👍

@malarzm
Copy link
Member

malarzm commented Dec 6, 2020

I've opened #2258 to track persistent collection changes

@malarzm
Copy link
Member

malarzm commented Dec 6, 2020

I have merged 2.1.x branch into the master recently so you'll need to rebase your branch.

@alcaeus alcaeus mentioned this pull request Dec 16, 2020
@alcaeus alcaeus requested a review from malarzm December 23, 2020 10:23
@alcaeus
Copy link
Member Author

alcaeus commented Dec 23, 2020

@malarzm I waited long enough to take care of proxies as well: we can now generate proxies with 7.4 language features on PHP 8 by switching to friendsofphp/proxy-manager-lts. Proxy support for PHP 8 features is still outstanding pending an upstream fix.

Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

@malarzm malarzm merged commit be0941e into doctrine:master Dec 28, 2020
@alcaeus alcaeus deleted the php-8-support branch December 29, 2020 00:14
@@ -22,17 +22,17 @@
{ "name": "Andreas Braun", "email": "alcaeus@alcaeus.org" }
],
"require": {
"php": "^7.2",
"php": "^7.2 || ^8.0",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@malarzm can this PR be tagged so we can update to PHP 8?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll try to have a release in January :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No hurry, I'm happy just with a plan 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aggregation\Stage\Match class won't work with PHP8
7 participants