-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
bumped min version of Twig to 1.28 #20484
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
fabpot
commented
Nov 10, 2016
Q | A |
---|---|
Branch? | 2.7 |
Bug fix? | no |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | see twigphp/Twig#2236 |
License | MIT |
Doc PR | n/a |
81a69f8
to
418c04d
Compare
What about using this great feature in Form, the component that needs it most? They even define a method called renderBlock() |
* @return array | ||
* @return Twig_Template[] | ||
* | ||
* @deprecated not used anymore internally |
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.
shouldn't it throw a deprecation warning ?
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.
Agreed, but not in Symfony 2.7. This must be added after merging this to master
.
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.
Agreed with @xabbuh, but this class should probably be marked as being internal as well in master.
@@ -200,7 +200,7 @@ public function toolbarAction(Request $request, $token) | |||
return new Response($this->twig->render('@WebProfiler/Profiler/toolbar.html.twig', array( | |||
'position' => $position, | |||
'profile' => $profile, | |||
'templates' => $this->getTemplateManager()->getTemplates($profile), | |||
'templates' => $this->getTemplateManager()->getNames($profile), |
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.
getNames()
must become public
then
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.
sure :)
5c8bc22
to
73e1c06
Compare
@javiereguiluz Unfortunately the logic in the Form integration is complex and cannot use this technique as we are manually passing blocks to |
73e1c06
to
b8f7614
Compare
Updated based on the latest changes for Twig 1.28. |
👍 Status: Reviewed |
This PR was merged into the 2.7 branch. Discussion ---------- bumped min version of Twig to 1.28 | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | see twigphp/Twig#2236 | License | MIT | Doc PR | n/a Commits ------- b8f7614 bumped min version of Twig to 1.28
…etTemplates() (maidmaid) This PR was merged into the 4.0-dev branch. Discussion ---------- [WebProfilerBundle] Remove deprecated TemplateManager::getTemplates() | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20484 | License | MIT | Doc PR | / Commits ------- f8be69d Remove TemplateManager::getTemplates()