Skip to content
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

Added twig.loader definition to dic_tags #2005

Merged
merged 1 commit into from Dec 26, 2012

Conversation

dantleech
Copy link
Contributor

Documentation update for PR symfony/symfony#6171 - twig_loader_pass

weaverryan added a commit that referenced this pull request Dec 26, 2012
Added twig.loader definition to dic_tags
@weaverryan weaverryan merged commit 5d72b6f into symfony:master Dec 26, 2012
weaverryan added a commit that referenced this pull request Dec 26, 2012
@weaverryan
Copy link
Member

Hi Dan!

Thanks for the docs - and great feature!

Cheers!

@dantleech
Copy link
Contributor Author

Your welcome, cheers.

On Wed, Dec 26, 2012 at 01:33:43PM -0800, Ryan Weaver wrote:

Hi Dan!

Thanks for the docs - and great feature!

Cheers!


Reply to this email directly or view it on GitHub:
#2005 (comment)

ondrejmirtes pushed a commit to ondrejmirtes/symfony that referenced this pull request Nov 25, 2013
This PR was merged into the master branch.

Commits
-------

f48b22a Added configuration pass that adds Twig Loaders

Discussion
----------

[Twig] [DI Pass] Added configuration pass that adds Twig Loaders

Bug fix: [no]
Feature addition: [yes]
Backwards compatibility break: [no]
Symfony2 tests pass: [yes]
Todo: Documentation?
License of the code: MIT

- Defined new Chain loader service with symfony Filesystem loader added
  by default.
- Added compiler class which picks up any services tagged "twig.loader"
- If there are any instances of "twig.loader" the "twig.loader" alias is
  set to the Twig_Loader_Chain service ID instead of the filesystem
  loader.

I think I still like the explicitness of the other pull request, but I defer to your judgment :) This is certainly much easier for the developer.

---------------------------------------------------------------------------

by dantleech at 2012-12-03T08:31:08Z

Will update the PR later today / tomorrow

---------------------------------------------------------------------------

by dantleech at 2012-12-03T18:19:09Z

ok, updated. I throw a `Symfony\Component\DependencyInjection\Exception\LogicException` if there are no loaders -- not sure if that is the best thing to do, or if that is the best exception.

---------------------------------------------------------------------------

by fabpot at 2012-12-05T15:28:24Z

Looks good to me. Can you add a note in the CHANGELOG about this new possibility and update the documentation accordingly? Thanks.

---------------------------------------------------------------------------

by dantleech at 2012-12-05T17:50:37Z

ok. updated change log and changed both count() comparisons to be strict. @fabpot which documentation should I update? or should I add `cookbook/templating/registering_multiple_loaders.rst`?

---------------------------------------------------------------------------

by stof at 2012-12-05T20:07:37Z

@dantleech at least the DIC tags reference need to be updated to mention the new tag. I'm not sure a dedicated article is needed for it (but @weaverryan will decide if it is worth it)

---------------------------------------------------------------------------

by dantleech at 2012-12-06T17:57:20Z

Made a PR for documentation: symfony/symfony-docs#2005

---------------------------------------------------------------------------

by dantleech at 2012-12-07T16:44:00Z

ok. have updated the correct CHANGELOG and squashed to one commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants