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] Fix BC break due required twig environment#24851
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
| private$computed; | ||
| publicfunction__construct(Profile$profile,Environment$twig) | ||
| publicfunction__construct(Profile$profile,Environment$twig =null) |
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.
Maybe passingnull should trigger a deprecation warning, so we can remove the boilerplate below on the 4.0 branch again.
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.
sounds reasonable :) lets see what others think.
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.
No need to "deprecate" that as we are talking about an additional feature.
| $template =$this->twig->load($name =$profile->getName()); | ||
| }catch (\Twig_Error_Loader$e) { | ||
| $template =null; | ||
| if (null !==$this->twig) { |
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.
if (null === $this->twig) {return;}?
about the deprecation, too late IMHO if we want to be strict about the feat freeze
nicolas-grekas commentedNov 7, 2017
Thank you@ro0NL. |
…o0NL)This PR was squashed before being merged into the 3.4 branch (closes#24851).Discussion----------[TwigBridge] Fix BC break due required twig environment| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes/no| Fixed tickets |#24236 (comment)| License | MIT| Doc PR | symfony/symfony-docs#... <!--highly recommended for new features-->Seeezsystems/ezplatform-design-engine#12 +silexphp/Silex-WebProfiler#126 +bolt/bolt#7154Sorry for that :)cc@fabpotCommits-------243e4b2 [TwigBridge] Fix BC break due required twig environment
GwendolenLynch commentedNov 8, 2017
Much appreciated@ro0NL 👍 |
Uh oh!
There was an error while loading.Please reload this page.
Seeezsystems/ezplatform-design-engine#12 +silexphp/Silex-WebProfiler#126 +bolt/bolt#7154
Sorry for that :)
cc@fabpot