[Config] Fix YamlReferenceDumper prototyped array support #19570
Conversation
| @@ -19,6 +19,9 @@ | |||
| "php": ">=5.3.9", | |||
| "symfony/filesystem": "~2.3" | |||
| }, | |||
| "require-dev": { | |||
| "symfony/yaml": "~2.7|~3.0" | |||
There was a problem hiding this comment.
Is this the proper versions constraint to use ?
There was a problem hiding this comment.
should be ~2.7 on 2.7,
~2.8|~3.0.0 on 2.8
and ~2.8|~3.0 on 3.1
should be done while merging up
There was a problem hiding this comment.
Thank you @nicolas-grekas . I'll update the PR.
| @@ -32,7 +31,7 @@ private function getConfigurationAsString() | |||
| acme_root: | |||
| boolean: true | |||
| scalar_empty: ~ | |||
| scalar_null: ~ | |||
| scalar_null: null | |||
There was a problem hiding this comment.
why is it dumped as null here while all other places using null are dumping it as ~ ?
There was a problem hiding this comment.
Because the ExampleConfiguration explicitly sets the default value to null.
When setting a default value, the YamlReferenceDumper will replace the default ~ by this value :)
(This is not related to the changes in this PR, though)
|
Any update about this one ? Should it rather be considered as a new feature as this implementation is missing since the introduction of the yaml dumper ? |
|
As the test is marked as skipped by mentioning specifically that this is not supported, I would say that this is indeed a new feature that needs to be merged in master. |
|
Can you update the PR accordingly? Thanks. |
|
Thank you @ogizanagi. |
…ort (ogizanagi) This PR was merged into the 3.2-dev branch. Discussion ---------- [Config] Fix YamlReferenceDumper prototyped array support | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Also related to #19480 which fixes another prototype issue, but cannot be tested properly on Travis because marked as skipped by this missing implementation. Previous output was: ```yaml [...] parameters: # Prototype name: ~ connections: user: ~ pass: ~ ``` instead of: ```yaml [...] parameters: # Prototype name: ~ connections: # Prototype - user: ~ pass: ~ ``` Commits ------- 063a980 [Config] Fix YamlReferenceDumper prototyped array support
…otypes (ogizanagi)
This PR was merged into the 3.2-dev branch.
Discussion
----------
[Config] Fix (Yaml|Xml)ReferenceDumper for nested prototypes
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
This tries to fix the nested prototypes case for the `YamlReferenceDumper` and `XmlReferenceDumper`.
### Before
```yml
cms_pages:
# Prototype
page: []
```
### After
```yml
cms_pages:
# Prototype
page:
# Prototype
locale:
title: ~ # Required
path: ~ # Required
```
~~However, the `YamlReferenceDumperTest::testDumper` is marked as skipped, due to another unsupported prototype usage, but that's another issue (the `connections` key). Thus, the bug fix must be tested manually :/ (I'd recommend to merge #19570 first)~~
Commits
-------
1e80510 [Config] Fix YamlReferenceDumper for nested prototypes
Also related to #19480 which fixes another prototype issue, but cannot be tested properly on Travis because marked as skipped by this missing implementation.
Previous output was:
instead of: