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

[DependencyInjection] fix ambiguous services schema#18246

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.3frombackbone87:ticket/17460
Apr 28, 2016

Conversation

@backbone87
Copy link
Contributor

QA
Branch?2.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?
Fixed tickets#17460#2122
LicenseMIT

@javiereguiluzjaviereguiluz changed the titlebug #17460 [DI] fix ambiguous services schema[DependencyInjection] fix ambiguous services schemaMar 21, 2016
@javiereguiluz
Copy link
Member

This issue seems very important. Could anyone who uses XML config verify it? Thanks! After all, people use XML because it validates the config ... so we must ensure that the validation is correct.

@xabbuh
Copy link
Member

Best imo would be to have additional tests that ensure that an XML file that wasn't valid before now is parsed properly.

@stof
Copy link
Member

stof commentedApr 7, 2016

@backbone87 can you add such a test ?

@backbone87
Copy link
ContributorAuthor

if the XmlFileLoader is validating the loaded xml, then the schema should be covered by its test cases already?

@backbone87
Copy link
ContributorAuthor

actually all the XmlFileLoader tests should fail without this ticket resolved, because the schema is invalid and therefore the xml cant be validated. i am not that familar with the validation behavior of libxml

@backbone87
Copy link
ContributorAuthor

ok i have played around with the test cases of the XmlFileLoader and it seems like libxml doesnt care about unique particle attribution and therefore "succeeds" validating the services.xml files with the old (invalid) schema.
in general i have noticed some wacky applications of the whole validation, but i will open another issue for it.

in general the existing testcases cover that this PR is working as intented.

@xabbuh
Copy link
Member

@backbone87 The thing is, if we cannot find an example that would fail without this change, there is no need to do it. So what I meant is that we need to find a failing example and have to add it on top of the already existing ones.

@backbone87
Copy link
ContributorAuthor

@xabbuh it cant fail, because libxml doesnt seem to care about the problem in the XSD, but every other standards conforming tool reports this XSD as invalid without the fix

the "benefit" from this PR is that you can now have your extension config between parameter and service config, which was invalid before:

<parameters>...</parameter><project:config>...</project:config><services>...</services>

@backbone87
Copy link
ContributorAuthor

i mean we could add a XSD linting test to the test suite, but that needs to use another XML tool than the ones bundled with PHP. every decent XSD editor lints automatically, so idk if its worth the hassle.

@backbone87
Copy link
ContributorAuthor

whats missing here? any opinions on the latest comments?

@xabbuh
Copy link
Member

The change itself looks good to me. 👍

But yeah I think we should use some external validation tool if that doesn't work properly with PHP.

@fabpot
Copy link
Member

Thank you@backbone87.

@fabpotfabpot merged commit9828f23 intosymfony:2.3Apr 28, 2016
fabpot added a commit that referenced this pull requestApr 28, 2016
…one87)This PR was merged into the 2.3 branch.Discussion----------[DependencyInjection] fix ambiguous services schema| Q             | A| ------------- | ---| Branch?       | 2.3| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   || Fixed tickets |#17460#2122| License       | MITCommits-------9828f23 bug#17460 [DI] fix ambiguous services schema
@fabpotfabpot mentioned this pull requestApr 29, 2016
This was referencedMay 9, 2016
@backbone87backbone87 deleted the ticket/17460 branchSeptember 29, 2016 21:06
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.

6 participants

@backbone87@javiereguiluz@xabbuh@stof@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp