Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DoctrineBridge][Routing] Require PSR-11 container instead of Symfony container#21978
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
nicolas-grekas commentedMar 11, 2017
To me, it should be ok to change even when a protected property is involved: existing code won't break all of a sudden because of the less restrictive type hint. In any doubt, just to it in another PR to have this one merged earlier is this needs to be discussed. |
enumag commentedMar 11, 2017
Alright, will do it in separate PR. Also I didn't bother with deprecated classes like ContainerAwareEventDispatcher. |
GuilhemN left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
👍 but the Routing component is no longer compatible with the DependencyInjection component <3.3 so it's requirements/conflicts should be updating.
enumag commentedMar 12, 2017
@GuilhemN Like this? |
GuilhemN left a comment
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, that's it :+1;
fabpot commentedMar 12, 2017
Thank you@enumag. |
… instead of Symfony container (enumag)This PR was merged into the 3.3-dev branch.Discussion----------[FrameworkBundle][TwigBundle] Require PSR-11 container instead of Symfony container| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |As discussed in#21978 here is the second PR.Commits-------857ce7c [FrameworkBundle][TwigBundle] Require PSR-11 container instead of Symfony container
Uh oh!
There was an error while loading.Please reload this page.
The changed classes can easily use PSR-11 container instead of normal symfony container.
I didn't change any classes that have the
$containerproperty as protected (for exampleSymfony\Bundle\FrameworkBundle\Translation\Translator) because I'm unsure whether that would be considered a BC break. Let me know if you want me to change such classes as well.