Skip to content

[Asset] ability to set assets_version_strategy as service #16522

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

[Asset] ability to set assets_version_strategy as service #16522

wants to merge 9 commits into from

Conversation

ewgRa
Copy link
Contributor

@ewgRa ewgRa commented Nov 11, 2015

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

While I working on #14832 I realize that all this problems and hidden magic can be avoided, if we will have ability to set asset version strategy as service.

This PR implementation of this idea.

Now it is possible to do something like this:

framework:
    assets:
        version_strategy: assets.custom_version_strategy
        base_urls: http://cdn.example.com
        packages:
            foo:
                base_urls: ["https://example.com"]
                version_strategy: assets.custom_version_strategy

There is can be some conflicts with #16511, I will fix it, depends on which one PR will be merged first (or not merged) :)

@ewgRa ewgRa mentioned this pull request Nov 11, 2015
Closed
@ewgRa
Copy link
Contributor Author

ewgRa commented Nov 11, 2015

Test fail seems not related to this PR

@ewgRa ewgRa changed the title [Asset] ability to set assets_version_strategy [Asset] ability to set assets_version_strategy as service Nov 11, 2015
@ewgRa
Copy link
Contributor Author

ewgRa commented Dec 9, 2015

ping @fabpot, what do you think? I think it is great feature

@xabbuh xabbuh added the Asset label Dec 9, 2015
ewgRa added 4 commits January 24, 2016 21:56
@ewgRa
Merge remote-tracking branch 'upstream/2.8' into assets_version_strategy
d52142f 
Conflicts:
	src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
	src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
	src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php
	src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/legacy_templating_assets.php
	src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml
	src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/legacy_templating_assets.xml
	src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml
	src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/legacy_templating_assets.yml
	src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
@ewgRa
add test
0360a28
@ewgRa
cs
1cffdb2
@ewgRa
Merge remote-tracking branch 'upstream/2.8' into assets_version_strategy
00bc205
@fabpot
Copy link
Member

fabpot commented Jan 25, 2016

As this is a new feature, it should be done on master instead. @ewgRa Can you rebase and submit a new PR? Thanks.

@ewgRa
Copy link
Contributor Author

ewgRa commented Jan 25, 2016

Closed in favor #17532

@ewgRa ewgRa closed this Jan 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Asset Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ewgRa @fabpot @xabbuh @carsonbot