-
-
Notifications
You must be signed in to change notification settings - Fork 188
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
Allow PHP 8 #628
Allow PHP 8 #628
Conversation
A recent enough version is needed for compatiblity with PHP 8
Blocked by laminas/laminas-code#46 I suppose 😬 |
Yes, and generally needs a lot of testing around PHP 8 features. |
That version is compatible with PHP 8
We need one that has support for PHP 8, so let's pick the latest.
The blocker is |
c9be49d
to
acc643c
Compare
Hurdle removed: https://github.com/beberlei/assert/releases/tag/v3.3.0 |
Version 3 has a requirement on webmozart/glob, which is not yet compatible with PHP 8
0ef6f97
to
8be196b
Compare
Aaaaand it circles back to Doctrine: UPD: fixed |
BTW, I don't mind testing with |
Ah I never used that flag. Let's try it and see if tests pass at all, then we can see what tests need to be added. |
Many libs might be compatible although they do not advertise it, using that flag will allow us to see if tests pass and to add more tests in the meantime.
3.5.0 is compatible with PHP 8
6f07af2
to
6d4cec0
Compare
It should be the easiest one.
6d4cec0
to
06cb824
Compare
I think this needs to be updated to account for union types: https://github.com/laminas/laminas-code/blob/40919916c4ca6ad815874539cf528814429f0269/src/Generator/TypeGenerator.php#L58-L59 |
Yup, see laminas/laminas-code#47 (comment) |
Waiting on laminas/laminas-code#57 |
Is there anything I can do to help? we'd really like to upgrade to version 8 before christmas. |
@volga you can help by writing tests of codegen for proxies that require PHP 8.0. Also need to try this patch against laminas/laminas-code#64 I can tell you from my end that I don't expect to release for xmas, since I am closing off other client work too. |
Also see #635 for "why is PHP8 support not here yet?" :-) |
@Ocramius @volga I tried this patch against laminas code 4.x in #637. Does it look good? I guess after laminas-code 4.0 is released we could make the required changes in ProxyManager to fix PHPstan and other issues reported by CI. I'd be happy to help with tests but unfortunately, I don't really get what is required. |
@Ocramius could you just confirm that now allowing php8 isn't because ProxyManager can't run on php8 but because it cannot process php8 features, ie. if I have a codebase that only uses php7 features then ProxyManager works? |
Correct, hence it cannot be allowed on PHP 8 yet. Be aware that PHP 8 already includes type refinements in core symbols out of the box. |
What do you mean by "Be aware that PHP 8 already includes type refinements in core symbols out of the box."? This refers to static return type + union types? |
Correct: there are already such types when you switch to PHP 8 :) If you need to install this package with PHP 8 and you take the risks upon yourself, run with |
If I have a php7.4 codebase and just want run it on php8 then there should be no problems with the ProxyManager? Ps: I don't mind waiting just want to confirm my understanding |
In theory nothing else: in practice, it needs to be tested. |
Obviously. Thank you for taking some time to answer my questions, and good work with this package and other open source project. Your Doctrine Best practices presentation were especially impactful. Too bad I understood them only after making a mess :). |
…d classes with PHP 8.0 types
Currently picking this up for local merging and review in #658 Indeed there were multiple PHP 8.0-related failures :-) |
Allow (and test) PHP 8.0 type compatibility
No description provided.