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

Add support for Symfony 7 where able #2400

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Dec 29, 2023

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.

@alexander-schranz
Copy link
Contributor

Hello @mbabker I just got today some time to Test @sulu which has no dependency to Sensio Extra Bundle against this pull request.

sulu/sulu#7272

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?

@mbabker
Copy link
Contributor Author

mbabker commented Feb 9, 2024

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 #[View] attribute/annotation processing and the event listener that converts a View returned from a controller into a Response on this PR since the attribute class is extending from a class in SensioFrameworkExtraBundle. So getting that one finished first lets me undo some of the changes I made in this PR to get a (mostly) passing CI.

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.

@DelphineCogi
Copy link

DelphineCogi commented Mar 14, 2024

Is there any update here ? 🙏

alexander-schranz added a commit to alexander-schranz/sulu that referenced this pull request Mar 18, 2024
alexander-schranz added a commit to alexander-schranz/sulu that referenced this pull request Mar 18, 2024
@TheDevick
Copy link

This PR is almost 5 months old and still not merged. Any updates about that?

@mbabker
Copy link
Contributor Author

mbabker commented Apr 1, 2024

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.

@mbabker mbabker force-pushed the sf-7 branch 3 times, most recently from d60109b to 20ebe35 Compare April 3, 2024 16:47
@mbabker mbabker force-pushed the sf-7 branch 9 times, most recently from 7f47513 to 6477622 Compare April 4, 2024 13:58
Co-authored-by: Alexander Schranz <[email protected]>
@alexander-schranz
Copy link
Contributor

Looks good from my side. @mbabker as you removed the draft this merge request is so not longer WIP?

@W0rma
Copy link
Contributor

W0rma commented Apr 4, 2024

I tested this branch in one of my projects and it worked fine. @mbabker Thank you for the hard work!

@mbabker mbabker changed the title [WIP] Add support for Symfony 7 where able Add support for Symfony 7 where able Apr 4, 2024
@mbabker
Copy link
Contributor Author

mbabker commented Apr 4, 2024

@mbabker as you removed the draft this merge request is so not longer WIP?

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.

@goetas goetas merged commit fa0f68b into FriendsOfSymfony:3.x Apr 4, 2024
11 checks passed
@goetas
Copy link
Member

goetas commented Apr 4, 2024

Thanks for the exceptional quality of work! Released as 3.7.0 https://github.com/FriendsOfSymfony/FOSRestBundle/releases/tag/3.7.0

@alexander-schranz
Copy link
Contributor

Thx @goetas and thank you @mbabker good work 💪

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

Successfully merging this pull request may close these issues.

6 participants