Remove templating dependency #213
Conversation
pyrech
force-pushed
the
remove-templating-dependency
branch
4 times, most recently
from
28c91a7
to
861c229
Compare
|
Generating json with a template engine seems weired alltogether. in particular: |
|
I know that symfony wanted to use Twig by default because of the speed. @stof can you confirm this? Is it a good practice to replace the template engine for twig for speed? |
|
Yep, using Twig directly instead of the whole templating component is probably a small optimization in terms of speed because of the many classes avoided. |
composer.json
Outdated
| "symfony/templating": "~2.7|~3.1", | ||
| "symfony/translation": "~2.7|~3.1" | ||
| "symfony/translation": "~2.7|~3.1", | ||
| "twig/twig": "~1.0|~2.0" |
There was a problem hiding this comment.
I suggesting depending on TwigBundle instead (as this bundle actually depends on services defined in the bundle, not just on Twig)
|
@monteiro the big benefit of this PR is indeed compatibility with Symfony 4, where the templating component would not be part of projects by default anymore (users would have to require it explicitly if they want to use it) |
pyrech
force-pushed
the
remove-templating-dependency
branch
3 times, most recently
from
0244e30
to
9286058
Compare
|
@stof thanks for the clarification. |
|
@pyrech let me see what is happening with the tests, since in master they failed as well. |
|
@pyrech can you plz rebase with master so we can have the tests green? Thanks! |
pyrech
force-pushed
the
remove-templating-dependency
branch
from
9286058
to
f1e4245
Compare
|
Done 😉 |
|
@pyrech it seems that the tests are failing for PHP 5.3 😞 Since in master passes, is it a problem with this PR? |
|
Hum, looks like a problem with translation ordering but no idea where could it come from 😕 |
|
@pyrech tested locally with php5.3 and it worked 😢 travis not. |
|
@pyrech can you rebase with master. I have dropped php 5.3 support in travis. |
pyrech
force-pushed
the
remove-templating-dependency
branch
from
f1e4245
to
8cd0636
Compare
|
Done, sorry for the delay |
|
@pyrech thanks! |
This PR removes the dependency on symfony/templating component and uses Twig directly
Ref #200