-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[TwigBundle] Deprecating support for legacy templates directories #28891
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
bb92d3d
to
4ad6e4a
Compare
@@ -115,14 +116,18 @@ public function load(array $configs, ContainerBuilder $container) | |||
} | |||
|
|||
if (file_exists($dir = $container->getParameter('kernel.root_dir').'/Resources/views')) { | |||
if ($dir !== $defaultTwigPath) { | |||
@trigger_error(sprintf('"%s" This templates directory is deprecated since 4.2, use "%s" instead.', $dir, $defaultTwigPath), E_USER_DEPRECATED); |
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.
- "%s" This templates directory is deprecated since 4.2, use "%s" instead.
+ Templates directory "%s" is deprecated since 4.2, use "%s" instead.
? (same below)
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.
Hey! fixed, thanks.
4ad6e4a
to
4f5ed6d
Compare
Ready! (AppVeyor failure is unrelated) |
4f5ed6d
to
e0da5ac
Compare
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.
after this we can safely remove the Resources/views
paths in TemplateIterator
and TemplateFinder
, correct? Perhaps add a @deprecated
mention here so we don't forget it.
What about TemplateReference
?
src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php
Show resolved
Hide resolved
Yeah, in 5.0 :)
It's part of the Templating layer of the FB, no related IMO. |
src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php
Outdated
Show resolved
Hide resolved
e0da5ac
to
acae8bb
Compare
@chalasr comments addressed, thanks. (Ready) |
src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php
Outdated
Show resolved
Hide resolved
acae8bb
to
8b390f3
Compare
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.
LGTM
Thank you @yceruto. |
…directories (yceruto) This PR was merged into the 4.2-dev branch. Discussion ---------- [TwigBundle] Deprecating support for legacy templates directories | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#10500 go ahead with #28810 (comment) - [x] Fix tests Commits ------- 8b390f3 Deprecating support for legacy templates directories
…uto) This PR was merged into the master branch. Discussion ---------- Deprecating support for legacy templates directory Update according to symfony/symfony#28891 Commits ------- a90ae14 Deprecating support for legacy templates directory
…om legacy directories (yceruto) This PR was merged into the 5.0-dev branch. Discussion ---------- [TwigBundle] Removed capability to load Twig template from legacy directories | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Ref #28891 Commits ------- 4b6752b Removed capability to load Twig template from legacy directories
go ahead with #28810 (comment)