Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Make Controller helpers final#24407
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
fabpot commentedOct 3, 2017
Thank you@nicolas-grekas. |
…-grekas)This PR was merged into the 3.4 branch.Discussion----------[FrameworkBundle] Make Controller helpers final| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -I propose to make all ControllerTrait methods final so we can add type hints.I also propose to add ControllerTrait::has/get so that AbstractController also has the methods.This will help move from Controller to AbstractController.Commits-------bbc52a1 [FrameworkBundle] Make Controller helpers final
ossinkine commentedOct 28, 2017
@nicolas-grekas Could you please explain what is the sense of making this methods final? |
nicolas-grekas commentedOct 28, 2017
@ossinkine see PR description. Why would you need to extend them? |
ossinkine commentedOct 28, 2017
@nicolas-grekas PR description just says you propose to make them final but does not say why. abstractclass MyControllerextends Controller{publicfunctionrender($view,array$parameters = [],Response$response =null) {$parameters =array_merge(['foo' =>'bar'],$parameters);returnparent::render($view,$parameters,$response); }} So children controllers continue to use By the same logic, other methods can be overridden |
nicolas-grekas commentedOct 28, 2017
@ossinkine after#24722, the method will have a new signature (type-hints, as stated in the above description), so that if your code extends the methods, it will break. That's why. |
ossinkine commentedOct 30, 2017
@nicolas-grekas I understand the new signature breaks BC and I've have to fix my code. But why I can't override the methods in future when my method signature is fixed? |
TeLiXj commentedOct 31, 2017
I'm surprised after try 3.4 beta 2 and discover this like ossinkine. |
I propose to make all ControllerTrait methods final so we can add type hints.
I also propose to add ControllerTrait::has/get so that AbstractController also has the methods.
This will help move from Controller to AbstractController.