Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[TwigBridge] Late deprecation for TwigRendererEngine::setEnvironment()#21021
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 commentedDec 22, 2016
Not sure about this one. AFAIU, the user will now receive two deprecation notices as if they don't pass Twig env as a constructor argument, they probably call the setter. |
stof commentedDec 22, 2016
well, if they pass it in the constructor and still call the setter later, removing the setter would break their app without warning. So we need a deprecation warning there (to avoid the double deprecation warning, we may trigger it only when we already have an environment) |
28d55d1 to5ef1565Comparechalasr commentedDec 22, 2016
Indeed, updated to avoid the double warning. Should this target 3.2? If so I'll update the corresponding UPGRADE file |
fabpot commentedDec 22, 2016
3.3 is fine. |
| has been removed. Inject it into the`TwigRendererEngine` instead. | ||
| * The`TwigRendererEngine::setEnvironment()` method has been removed. | ||
| Pass the Twig Environment as second argument of the constructor instead. |
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.
Should also be documented in theUPGRADE-3.3.md file.
5ef1565 toaabb73cComparechalasr commentedDec 22, 2016
UPGRADE files updated |
xabbuh commentedDec 22, 2016
👍 |
fabpot commentedDec 23, 2016
Thank you@chalasr. |
…tEnvironment() (chalasr)This PR was merged into the 3.3-dev branch.Discussion----------[TwigBridge] Late deprecation for TwigRendererEngine::setEnvironment()| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets |#20093 (comment)| License | MIT| Doc PR | n/aThis method should have been deprecated in 3.2 since the twig environment should be injected through the constructor and replacing it later can break things (see#20093 (comment) for details).Maybe this could target 3.2Commits-------aabb73c Deprecate TwigRendererEngine::setEnvironment()
This method should have been deprecated in 3.2 since the twig environment should be injected through the constructor and replacing it later can break things (see#20093 (comment) for details).
Maybe this could target 3.2