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

[TwigBundle] Warmup twig templates in non-standard paths #14764

Closed
wants to merge 1 commit into from
Closed

[TwigBundle] Warmup twig templates in non-standard paths #14764

wants to merge 1 commit into from

Conversation

kbond
Copy link
Member

@kbond kbond commented May 27, 2015

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #12507
License MIT
Doc PR symfony/symfony-docs#5391

@@ -78,6 +78,8 @@ public function load(array $configs, ContainerBuilder $container)
$container->addResource(new DirectoryResource($path));
}

$container->setParameter('twig.paths', $config['paths']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not create parameters for such things. Instead, you need to inject the value directly via replaceArgument and add a comment in the XML about the value that is going to be injected (have a look at some other examples in the framework).

@kbond
Copy link
Member Author

kbond commented May 28, 2015

Those changes were made.

@@ -27,14 +29,16 @@ class TemplateCacheCacheWarmer implements CacheWarmerInterface
{
protected $container;
protected $finder;
protected $paths;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could it be private actually ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I typically would make it private but was following the convention of the file. Should I make it private then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes please. Any new property should start as being private. We only switch to protected when we decide it should be a supported extension point (and I have no idea why we made the other properties protected btw, but changing it is a BC break)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. This change was made.

@kbond
Copy link
Member Author

kbond commented May 29, 2015

Any chance this can make it into 2.7?

@jakzal
Copy link
Contributor

jakzal commented May 29, 2015

@kbond we don't accept new features in 2.7 anymore. It'll be released in few days.

@fabpot
Copy link
Member

fabpot commented Jun 11, 2015

@kbond Can you rebase on current 2.7 and fix the small CS issue?

@kbond
Copy link
Member Author

kbond commented Jun 11, 2015

Done.

@fabpot
Copy link
Member

fabpot commented Jun 11, 2015

👍

@xabbuh
Copy link
Member

xabbuh commented Jun 28, 2015

👍 (though I would rename findTemplatesInFolder() to findTemplatesInDirectory())

@fabpot
Copy link
Member

fabpot commented Jun 30, 2015

Thank you @kbond.

@fabpot fabpot closed this in cad16ac Jun 30, 2015
xabbuh added a commit to symfony/symfony-docs that referenced this pull request Oct 3, 2015
…paced twig templates (kbond)

This PR was merged into the 2.3 branch.

Discussion
----------

[Cookbook][Templating] Add note about cache warming namespaced twig templates

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | symfony/symfony#14764
| Applies to    | 2.3
| Fixed tickets | #5391

Commits
-------

cbd86ce add note about cache warming
@fabpot fabpot mentioned this pull request Nov 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants