update xml schema, fix #111 #113
Conversation
dbu
commented
Dec 27, 2013
dbu
commented
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #111 |
| License | MIT |
| Doc PR | - |
| @@ -27,35 +27,6 @@ public function getConfigTreeBuilder() | |||
|
|
|||
| $rootNode | |||
| ->children() | |||
| ->arrayNode('sonata_admin') | |||
There was a problem hiding this comment.
i moved this to the bottom of the Configuration file to be consistent with the sonata location in other configs.
|
|
||
| <xsd:complexType name="multilang"> | ||
| <xsd:sequence> | ||
| <xsd:element name="locale" type="xsd:NMTOKEN"/> |
There was a problem hiding this comment.
is this the right way to get
<multilang>
<locale>en</locale>
</multilang>
or should i do something different?
There was a problem hiding this comment.
you should use maxOccurs="unbounded" and I'm not so sure about xsd:NMTOKEN, seems like it's only there to be BC with XML 1.0. We may add our own locale type using the pattern [a-z]{2}, or we simply use xsd:string (I think I used that in the other bundles)
There was a problem hiding this comment.
changed to xsd:string and added unbounded, thanks
|
@dbu I'll merge symfony-cmf/Testing#33 within an hour, could you please add a test in your PR for this? |
|
oh, that would certainly help to have the test. could you do the test on one of the bundles so i see how its meant to be done? |
|
@dbu you forgot to push your latest changes, could you please do that, then I'll push the testing example here. |
|
@dbu fixed the things and added the test |
|
sorry, did not fixed the things :) |
|
okay, removed fixXmlConfig and pushed. lets see what the test thinks. |
|
Please don't merge this in the current state, the fixtures should be moved to resources/Fixtures |
|
you mean Tests/Resources/DataFixture => Tests/Resources/Fixtures? do you want to do that? i will be pretty much offline from tomorrow for a week, so feel free to move things and merge. |
|
Yes, I can do that but I don't have acces to a computer now |
|
@wouterj ok to merge? |
|
@lsmith77 ready now |
|
Thank you! |
