-
-
Notifications
You must be signed in to change notification settings - Fork 920
[2.4] Allow autowiring of class extending SearchFilter #2643
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
Remove a BC break introduced in api-platform#2630
#2640 related? @meyerbaptiste can we merge this? |
No, it's not related. Did my change break the autowiring @antograssiot? Because apart from that it's not a BC break. 🤔 |
I'm also asking because it has been changed in the cs fixes you did in: 97fa954#diff-77ec54980605d6062780fcd93ba31c64 :p |
@soyuka No #2640 is not related and on master only. |
But a default value here is a nonsense because the following argument is not optional ( |
I know/understand that it is a "non sense" to have this optional value before mandatory arguments but we can't change the argument order either. I don't have a better proposal
|
Ahah, yes I agree with that and your patch regarding the autowiring. Maybe the DependencyInjection component should inject |
My bad, thx @antograssiot! |
Now it feels like we're writing code for the autowiring lol! |
Do we have a way ( Anyway, I'd strongly argue that breaking autowiring is NOT a BC break. If it's about |
Alternative fix to #2643: allow autowiring of filter classes
Remove a BC break introduced in #2630