[FrameworkBundle] Fix MicroKernelTrait for php 8 #36943
Conversation
47b908f to
5e44cb4
Compare
5e44cb4 to
7476a16
Compare
There was a problem hiding this comment.
Nice improvements thanks, just some nitpicking.
I've noticed that implementing an old-style configureContainer() does not raise any deprecation. Is that on purpose? I really think that we should deprecate the old way, so we can re-add the abstract methods again in Symfony 6.
Yes, that's on purpose: my opinion on this is that the is a nice extension point to easily allow extensibility of the DSL. We're fine is ppl want to use any variant of it (by taking inspiration of the one we added in 5.1).
The previous DSL is still fine, it's not deprecated and shouldn't be to me.
src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/MicroKernelTraitTest.php
Outdated
Show resolved
Hide resolved
7476a16 to
ede0502
Compare
|
Ladies and gentlemen, attached to this PR, you can witness the first green php 8 build of the 5.1 branch. 🎉 |
cdef8b7 to
7f3132e
Compare
|
Thank you @derrabus. |
|
And now master \o/ Thanks for your hard work @derrabus!!! |
This PR fixes several php 8 related issues with
MicroKernelTrait.TypeErrorsraised by php. That code broke because the wording of those errors has changed. I've replaced that logic with a hopefully less brittle reflection-based approach.I've noticed that implementing an old-style
configureContainer()does not raise any deprecation. Is that on purpose? I really think that we should deprecate the old way, so we can re-add the abstract methods again in Symfony 6.