Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[2.7][Asset] Ability to set empty version strategy in packages#16511

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:2.7fromewgRa:issue-14832
Jan 24, 2016
Merged

[2.7][Asset] Ability to set empty version strategy in packages#16511

fabpot merged 1 commit intosymfony:2.7fromewgRa:issue-14832
Jan 24, 2016

Conversation

@ewgRa
Copy link
Contributor

QA
Bug fix?yes
New feature?no
BC breaks?no, but not sure, as for me test was wrong
Deprecations?no
Tests pass?yes
Fixed tickets#14832
LicenseMIT
Doc PR

Comment about '' thing. As you can see in xml test - we can't for attribute set null value. And for xml version '' empty string considered as null value, that affect all others formats and test is changed.

@ewgRaewgRa changed the titleIssue 14832[2.7][Asset] Ability to set empty version strategy in packagesNov 10, 2015
@xabbuh
Copy link
Member

Won't the version benull when you omitted theversion attribute in the XML format (instead of setting it to an empty string)?

@ewgRa
Copy link
ContributorAuthor

There is a case, if version is omitted - then default strategy used.

If we change it and if in xml omitted version considered as empty strategy (also as we need sync same logic for php, yml then) - we will have broken BC.

There is also legacy code (framework templating configuration) that have different logic - omitted considered as empty.

@ewgRa
Copy link
ContributorAuthor

There is no possible to set attribute in xml to null as far as I know :(

@XWB
Copy link
Contributor

XWB commentedNov 24, 2015

Ping@fabpot Any feedback on this? This bug was introduced with Symfony 2.7 and has not been resolved yet.

@ewgRa
Copy link
ContributorAuthor

ping@fabpot@xabbuh any comments?

@xabbuhxabbuh added the Asset labelJan 23, 2016
@fabpot
Copy link
Member

👍

@ewgRa Can you rebase to get rid of the 2 merge commit that prevents me from merging? Or even better, squash all commits. Thanks.

@ewgRa
Copy link
ContributorAuthor

@fabpot done

@fabpot
Copy link
Member

Thank you@ewgRa.

@fabpotfabpot merged commit646fc9c intosymfony:2.7Jan 24, 2016
fabpot added a commit that referenced this pull requestJan 24, 2016
…ages (ewgRa)This PR was merged into the 2.7 branch.Discussion----------[2.7][Asset] Ability to set empty version strategy in packages| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no, but not sure, as for me test was wrong| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#14832| License       | MIT| Doc PR        |Comment about '' thing. As you can see in xml test - we can't for attribute set null value. And for xml version '' empty string considered as null value, that affect all others formats and test is changed.Commits-------646fc9c Ability to set empty version strategy in packages
@ewgRaewgRa deleted the issue-14832 branchJanuary 24, 2016 11:44
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@fabpot Sorry, I accidently omit defaultNull here - fix#17514

fabpot added a commit that referenced this pull requestJan 25, 2016
This PR was merged into the 2.7 branch.Discussion----------[2.7][Asset] Add defaultNull to version configuration| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#14832,#16511| License       | MIT| Doc PR        |In#16511 PR was omitted defaultNull version for one case.Commits-------65adb72 add defaultNull to version
@ewgRaewgRa mentioned this pull requestJan 25, 2016
fabpot added a commit that referenced this pull requestJan 26, 2016
This PR was squashed before being merged into the 3.1-dev branch (closes#17532).Discussion----------[Asset] Version as service| 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:```yamlframework:    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 when it will be in masterCommits-------52d116b [Asset] Version as service
@fabpotfabpot mentioned this pull requestFeb 3, 2016
This was referencedFeb 28, 2016
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ewgRa@xabbuh@XWB@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp