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

Drop symfony4 support #21

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sral97
Copy link

@sral97 sral97 commented Jan 2, 2023

  • Use new release of php-rest-api
  • Remove dev-dependency to sensio/framework-extra-bundle
  • Upgrade other dependencies as well

@Edwin-Luijten
Copy link
Contributor

Thanks a bunch @sral97 , I tested your pull request and saw that the required monolog version requires 8.1+.

IMO It's safe to bump the version of php to 8.1 (8.0 is EO Security fixes only and EOL in 10 month's anyway).
It's also possible to support both 8.0 and 8.1, only need to apply a second version constraint on monolog, that supports 8.0.

Also, just like with the php-rest-api package it's best to also support symfony ^6.

I'm also thinking of dropping the suggestion of using the jms/serializer-bundle.
And I will change from travis to github workflow, updating the php version matrix to use supported versions after your contributions are merged (Thanks again).

@sral97
Copy link
Author

sral97 commented Jan 25, 2023

I didn't add the symfony 6 support in this PR because that would require to update the functional tests to work with symfony 6. I would treat that as a seperate issue/PR, as I was not sure if the kernel can be written in a way to support both symfony 5 and 6. I should be able to look into this sometime this week.

@sral97
Copy link
Author

sral97 commented Jan 25, 2023

Good news: it looks like all that was needed to get it working with both symfony versions was to drop the explicit route loading in the kernel and just rely on the one provided by each versions' MicroKernelTrait.

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

2 participants