Skip to content

[FrameworkBundle] remove default null value for asset version #17605

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

Merged
merged 1 commit into from
Jan 30, 2016

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jan 30, 2016

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

Setting null as the version of a package means that it uses the empty
version strategy. However, omitting the version option entirely was
meant to fall back to the default version strategy. This is not possible
when the default version value is null as there is no way to remove
it.

Setting `null` as the version of a package means that it uses the empty
version strategy. However, omitting the `version` option entirely was
meant to fall back to the default version strategy. This is not possible
when the default version value is `null` as there is no way to remove
it.
@xabbuh
Copy link
Member Author

xabbuh commented Jan 30, 2016

This will make tests of the FrameworkBundle pass again.

@ewgRa Can you have a look here as you initially added the default value in #17514 and I want to make sure not to break anything here which you intended to fix?

@xabbuh
Copy link
Member Author

xabbuh commented Jan 30, 2016

There are more broken tests in the FrameworkBundle which are not related to the Asset component. :(

@xabbuh xabbuh mentioned this pull request Jan 30, 2016
@fabpot
Copy link
Member

fabpot commented Jan 30, 2016

Thank you @xabbuh.

@fabpot fabpot merged commit 25f735f into symfony:2.7 Jan 30, 2016
fabpot added a commit that referenced this pull request Jan 30, 2016
…ion (xabbuh)

This PR was merged into the 2.7 branch.

Discussion
----------

[FrameworkBundle] remove default null value for asset version

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

Setting `null` as the version of a package means that it uses the empty
version strategy. However, omitting the `version` option entirely was
meant to fall back to the default version strategy. This is not possible
when the default version value is `null` as there is no way to remove
it.

Commits
-------

25f735f remove default null value for asset version
@xabbuh xabbuh deleted the default-assets-version-strategy branch January 30, 2016 16:48
@fabpot fabpot mentioned this pull request Feb 3, 2016
@ewgRa
Copy link
Contributor

ewgRa commented Feb 12, 2016

@xabbuh sorry for late reply, I miss this issue

My original commit was related to https://github.com/symfony/symfony/pull/16511/files#r50645726.
When I work on PR I miss defaultNull and in next fix-PR restore it.

Seems if tests working and "fall back to the default version strategy" is covered, than your fix right, but on first look seems it is broke BC.

@xabbuh
Copy link
Member Author

xabbuh commented Feb 12, 2016

@ewgRa I'll happily look if we need to fix anything if you happen to find a configuration that used to work before and now doesn't work anymore.

This was referenced Feb 28, 2016
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.

4 participants