-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
Fix symfony 7 compatibility #116
base: main
Are you sure you want to change the base?
Conversation
ff4faca
to
4f067d9
Compare
Is there anything left to do? This blocks symfony 7 updates :( |
Friendly ping @nicolas-grekas maybe (?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes great thanks.
Two ✅ Can be merge this @nicolas-grekas ? |
@core23 Thanks for working on the symfony 7 compatibility. I stumbled upon this and saw that checks are failing due to deprecations. I was able to fix them locally. I could provide a separate PR, but wouldn't it be better to provide the code to you? Should I create a PR in your fork which you then can merge in your symfony7 branch? |
AFAIK they are not related to this PR, so it would be better to fix this deprecation issues on the main project first. |
I will do that, thanks. |
@core23 I just had a look. As the current tagged version is 3.3.3, which bumped Symfony up to 7, I cannot fix the deprecation warnings without fixing the return type declarations too, which are part of your PR :) |
@nicolas-grekas Hi Nicolas, could you maybe help out? How should we proceed with the deprecation warnings? I could either bump up the code to the current implementation, but then the lowest versions are not supported anymore. Or do we disable deprecations on the highest versions? |
Fixes #114 by providing the correct method signature, depending on the loaded
symfony/security-core
version.Might close: #112 #115 #114