-
Notifications
You must be signed in to change notification settings - Fork 702
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 Symfony 7 where able #2400
Conversation
ff566ee
to
18b37b8
Compare
Hello @mbabker I just got today some time to Test @sulu which has no dependency to Sensio Extra Bundle against this pull request. Sulu itself still on Symfony 6.4 but I think it is good to know that all still works like expected also on Sulu side where the Extra Bundle is not installed. And so not any accidently bc break was introduced here which would trigger an error on such project like Sulu. Sulu uses the JMS Serializer inside FOSRest and all seems still work there also like expected. @mbabker What do you think is required to check to move this forward? |
I think for me, in priority order, the PRs to focus on are:
I treat #2401 as a blocker to this because I basically disabled the whole Finishing this PR up after that gets to a good "we're most of the way there" milestone. #2398 is probably the hardest to deal with because it is impossible to make a one-for-one replacement of the existing behavior. But at the same time, it's already an optional integration, so I wouldn't let the param converter not being Symfony 7 compatible block shipping an update that adds Symfony 7 support for all other features in this bundle, and the folks relying on that can continue to use the Symfony 6.4 LTS for a little while longer. |
Is there any update here ? 🙏 |
This PR is almost 5 months old and still not merged. Any updates about that? |
This specific pull request isn't moving forward until its blocker, #2401, is addressed. From my end, nothing has been forgotten about here and there is no need for the "updates please?" comments. |
d60109b
to
20ebe35
Compare
7f47513
to
6477622
Compare
Co-authored-by: Alexander Schranz <alexander@sulu.io>
Looks good from my side. @mbabker as you removed the draft this merge request is so not longer WIP? |
I tested this branch in one of my projects and it worked fine. @mbabker Thank you for the hard work! |
I knew I forgot something when updating everything else in this PR 😆 But, yeah, aside from the one major feature callout, this should be G2G for use with Symfony 5.4, 6.4, and 7.0. |
Thanks for the exceptional quality of work! Released as 3.7.0 https://github.com/FriendsOfSymfony/FOSRestBundle/releases/tag/3.7.0 |
This PR upgrades most of the bundle to support Symfony 7. Also wrapped into this PR are:
The one feature not covered is the request body param converter, which has its own dedicated PR to deal with implementing an argument resolver replacing that.