8000 Minor fixes to the FrameworkBundle configuration by wouterj · Pull Request #11765 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Minor fixes to the FrameworkBundle configuration #11765

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 4 commits into from
Closed

Minor fixes to the FrameworkBundle configuration #11765

wants to merge 4 commits into from

Conversation

wouterj
Copy link
Member
@wouterj wouterj commented Aug 25, 2014
Q A
Bug fix? yes
New feature? no
BC breaks? no (yes, but it was never used, since it never worked)
Deprecations? yes
Tests pass? yes
Fixed tickets -
License MIT
Doc PR symfony/symfony-docs#4121

@wouterj wouterj mentioned this pull request Aug 25, 2014
Closed
@stof
Copy link
Member
stof commented Aug 25, 2014

BC breaks? no (yes, but it was never used, since it never worked)

Wrong. It was working, because the Configuration class currently accepts strings for trusted_proxies and explode them into on comma, meaning the following works:

<config trusted-proxies="127.0.0.1, 10.0.0.1/24">
</config>

@wouterj
Copy link
Member Author
wouterj commented Aug 25, 2014

Ah, thanks @stof. I missed that. I'll put the attribute back in the XSD, or should I remove the element completely?

@stof
Copy link
Member
stof commented Aug 25, 2014

well, please add a test using each of them to see whether it works in a BC way (fixXmlConfig might create issues with the attribute exploding depending on the order in which they are processed). If both work, it is fine to keep the element (and marking the attribute as deprecated)

wouterj
wouterj reviewed Aug 30, 2014
View reviewed changes
src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd
@@ -8,7 +8,9 @@
<xsd:element name="config" type="config" />

<xsd:complexType name="config">
<xsd:all>
<xsd:sequence>
Copy link
Member Author

Choose a reason for hiding this comment

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

this is a major BC break, since elements now must have a predefined order. However, within xsd:all, all elements are either available or not, but they can't occur multiple times.

@fabpot
Copy link
Member

What's the status of this PR? I err on the side of closing it as it breaks BC.

@fabpot fabpot closed this Sep 23, 2014
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
FrameworkBundle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wouterj @stof @fabpot
0