-
-
Notifications
You must be signed in to change notification settings - Fork 52
Fix Symfony 7 compatibility #121
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
Conversation
d26b452 to
b7646a9
Compare
b7646a9 to
29379e6
Compare
|
friendly ping @nicolas-grekas @OskarStark |
|
Can you take a look at this one? currently is blocking update for some bundles using security-acl and the fix is not complex :) Thank you |
Co-authored-by: Nicolas Grekas <[email protected]>
|
Any news about a release with this PR? This is blocking my update to SF7. |
Really! I'm also in the same situation, help us. |
| */ | ||
| trait AclVoterTrait | ||
| { | ||
| public function vote(TokenInterface $token, mixed $subject, array $attributes): int |
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.
the way to support multiple versions without such trait is actually to add the return type without adding the parameter type, thanks to variance rules.
however, note that adding the return type requires releasing this as a major version of the package, as this class is not final or internal.
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.
IMO, since this major version was previously tagged as it supports sf 7, we should fix it here, and in the next major remove all the unnecessary code and leave the voter clean again. wdyt? I see it more like bug fixing rather than adding new things.
|
Thank you @jordisala1991. |
|
New release done as well. |
Thank you. |
Continuation of #116
I want to see what is failing to fix it.
Might close: #112 #115 #114 #121