Skip to content

Conversation

@fabpot
Copy link
Member

@fabpot fabpot commented Feb 12, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Right now, the assets configuration is optional (and the dependency as well on FramewokrBundle), but the asset services and the Asset extension is always loaded. This PR fixes this inconsistency, which will make tests pass again.

@fabpot fabpot merged commit ddf5ac4 into symfony:2.7 Feb 12, 2015
fabpot added a commit that referenced this pull request Feb 12, 2015
This PR was merged into the 2.7 branch.

Discussion
----------

made the assets optional

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Right now, the assets configuration is optional (and the dependency as well on FramewokrBundle), but the asset services and the Asset extension is always loaded. This PR fixes this inconsistency, which will make tests pass again.

Commits
-------

ddf5ac4 made the assets optional
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be?

$container->getDefinition('twig.extension.assets')->addTag('twig.extension');

@stof
Copy link
Member

stof commented Feb 12, 2015

@fabpot there is a BC break here: currently the asset function is always available (and does not require any special config as it creates a default package based on the Request by default). With your change, a project which does not define asset-related settings would not get the function anymore. This means that the BC layer is incomplete

fabpot added a commit that referenced this pull request Feb 16, 2015
This PR was merged into the 2.7 branch.

Discussion
----------

[FrameworkBundle] Enable assets by default

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #13667
| License       | MIT
| Doc PR        | -

This should make the tests pass again.

There are two templates from TwigBundle that are used with functional tests, and require the asset() helper:
* src/Symfony/Bundle/TwigBundle/Resources/views/Exception/exception_full.html.twig
* src/Symfony/Bundle/TwigBundle/Resources/views/layout.html.twig

Commits
-------

f5c0a69 [FrameworkBundle] Enable assets by default.
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.

3 participants