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, require Symfony 5.4+ and PHP 7.4+ #937

Merged
merged 1 commit into from Dec 12, 2023

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Oct 6, 2023

Q A
Bug fix? no
New feature? yes
Doc updated no
BC breaks? technically yes
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT

Adds Symfony 7 support, WIP as this builds on top of #933 to ensure it works with the annotations support being removed.

Since the first push of this PR failed on the lowest deps build because of incompatible signatures, this will also drop support for Symfony versions before 5.4 (current LTS), and bumps the PHP minimum to 7.4 because according to the Packagist stats the number of folks running a 5.x release of the bundle on PHP 7.2 or 7.3 is so insignificant that at this point it's just more efficient to drop those EOL PHP branches.

@mbabker
Copy link
Contributor Author

mbabker commented Oct 6, 2023

The lowest deps build is showing that it might be time to start dropping legacy Symfony support. https://github.com/schmittjoh/JMSSerializerBundle/actions/runs/6435785419/job/17477761506?pr=937 fatal'd out because the signature of the cache warmer isn't compatible with Symfony 3.4, and it's typehinted in Symfony 7.

composer.json Outdated
"symfony/framework-bundle": "^3.4 || ^4.0 || ^5.0 || ^6.0"
"jms/serializer": "^3.28",
"jms/metadata": "^2.6",
"symfony/dependency-injection": "^3.4 || ^4.0 || ^5.0 || ^6.0 || ^7.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is crazy to see that we are able to run this bundle on a such large range of symfony versions

@mbabker mbabker changed the title [WIP] Add support for Symfony 7 [WIP] Add support for Symfony 7, require Symfony 5.4+ and PHP 7.4+ Oct 9, 2023
@mbabker mbabker force-pushed the symfony-7 branch 3 times, most recently from 3fe88b1 to 96779fc Compare November 3, 2023 23:02
@loic425 loic425 mentioned this pull request Nov 8, 2023
69 tasks
@OrestisZag
Copy link

@goetas @mbabker Thank you for tackling this issue. I see that the checks are green. Are we gonna merge this soon or is there something more to be done?

@l-you
Copy link

l-you commented Dec 4, 2023

@mbabker Is this PR ready to merge? Please request a contributor review if so.

@mbabker
Copy link
Contributor Author

mbabker commented Dec 4, 2023

The current state is reflected in the PR description.

WIP as this builds on top of #933 to ensure it works with the annotations support being removed

The dependent PR should be reviewed/merged separately, and because of the changes it makes, it is a blocker to allowing this one to be reviewed/merged.

@ruudk
Copy link
Contributor

ruudk commented Dec 8, 2023

The dependent PR has been merged 🎉 Could you rebase this one and prepare it for review? Thanks so much for your work on this 💙 💪

@mbabker mbabker marked this pull request as ready for review December 8, 2023 14:09
@mbabker mbabker changed the title [WIP] Add support for Symfony 7, require Symfony 5.4+ and PHP 7.4+ Add support for Symfony 7, require Symfony 5.4+ and PHP 7.4+ Dec 8, 2023
@chalasr
Copy link

chalasr commented Dec 9, 2023

Looks good to me. Could we have a merge here? SymfonyCon' hackday is happening now and we need this to fix other bundles' compatibility.

@goetas goetas merged commit 6fa2dd0 into schmittjoh:master Dec 12, 2023
6 checks passed
@mbabker mbabker deleted the symfony-7 branch December 12, 2023 15:36
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.

None yet

6 participants