[FrameworkBundle] [TwigBundle] Move the hinclude key away from templating #30959
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
c045b3d to
e6dfc08
Compare
fabpot
requested changes
Apr 7, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Show resolved
Hide resolved
stof
reviewed
Apr 7, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
e6dfc08 to
2957e52
Compare
2957e52 to
98c6732
Compare
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
98c6732 to
f53edb0
Compare
fabpot
reviewed
Apr 7, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Show resolved
Hide resolved
|
It does work properly since I didnt removed anything I just added the
deprecation
Le dim. 7 avr. 2019 à 18:53, Fabien Potencier <notifications@github.com> a
écrit :
… ***@***.**** commented on this pull request.
------------------------------
In
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
<#30959 (comment)>:
> @@ -487,6 +487,7 @@ private function registerFragmentsConfiguration(array $config, ContainerBuilder
return;
}
+ $container->setParameter('fragment.renderer.hinclude.global_template', $config['hinclude_default_template']);
Does it work even when providing the value via the old key and not via the
new one?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#30959 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADSq8hOp9EZCqrgg-gnArTKIx-YNs2wCks5veiJvgaJpZM4cgo9l>
.
|
|
Ooow I understand, I will write one !
Le dim. 7 avr. 2019 à 21:56, Robin Chalas <notifications@github.com> a
écrit :
… ***@***.**** commented on this pull request.
------------------------------
In
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
<#30959 (comment)>:
> @@ -487,6 +487,7 @@ private function registerFragmentsConfiguration(array $config, ContainerBuilder
return;
}
+ $container->setParameter('fragment.renderer.hinclude.global_template', $config['hinclude_default_template']);
@Simperfit <https://github.com/Simperfit> You are setting a parameter
with no prior existence/emptiness check at 2 different places so only one
can win (and potentially not the expected one). We need a test I guess :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#30959 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADSq8gQ9gFaNNDHFMwT6HHFKttPRNSDgks5vek1egaJpZM4cgo9l>
.
|
f53edb0 to
0a0d913
Compare
|
I've added an existence check to not replace if it has been already setted. |
fabpot
requested changes
Apr 8, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
3c89d11 to
2ee4331
Compare
|
@fabpot Should be ok now, I've added a test. |
fabpot
reviewed
Apr 8, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
2ee4331 to
400ed9a
Compare
|
@fabpot PR ready, CI failure not related to this PR. |
fabpot
reviewed
Apr 8, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
fabpot
requested changes
Apr 8, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
400ed9a to
7fb7329
Compare
|
done @fabpot |
7fb7329 to
2dfcc51
Compare
fabpot
reviewed
Apr 8, 2019
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
2dfcc51 to
9b605e9
Compare
|
done @fabpot |
fabpot
approved these changes
Apr 8, 2019
9b605e9 to
4f39339
Compare
|
Thank you @Simperfit. |
fabpot
added a commit
that referenced
this pull request
Apr 8, 2019
…way from templating (Simperfit) This PR was squashed before being merged into the 4.3-dev branch (closes #30959). Discussion ---------- [FrameworkBundle] [TwigBundle] Move the hinclude key away from templating | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | yes <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #30874 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | to do when pr is validated. <!-- Write a short README entry for your feature/bugfix here (replace this comment block.) This will help people understand your PR and can be used as a start of the Doc PR. Additionally: - Bug fixes must be submitted against the lowest branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the master branch. --> Maybe I shouldn't move directly the config key from templating to the other, but since the templating component has been deprecated we may change this directly without deprecating that key alone, WDYT ? Commits ------- 4f39339 [FrameworkBundle] [TwigBundle] Move the hinclude key away from templating
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Maybe I shouldn't move directly the config key from templating to the other, but since the templating component has been deprecated we may change this directly without deprecating that key alone, WDYT ?