8000 [Yaml] respect inline level when dumping objects as maps by goetas · Pull Request #22419 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Yaml] respect inline level when dumping objects as maps #22419

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

goetas
Copy link
Contributor
@goetas goetas commented Apr 13, 2017
Q A
Branch? 3.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #22409
License MIT
Doc PR

Alternative version of #22409 that addresses the issues expressed in #22409 (comment)

@goetas
Copy link
Contributor Author
goetas commented Apr 13, 2017
  • The changes in src/Symfony/Component/Yaml/Inline.php are: dumpArray has been split in two parts (dumpArray and dumpMap), and dumpMap is always used to dump objects when the DUMP_OBJECT_AS_MAP option is provided

  • The changes in src/Symfony/Component/Yaml/Dumper.php are less "nice looking", but are mainly about detecting when the DUMP_OBJECT_AS_MAP case should be considered and making sure that the foreach loop does what is supposed to do. I'm open for suggestions to make it more "nice looking"...

@stof stof requested a review from xabbuh April 13, 2017 13:27
@xabbuh
Copy link
Member
xabbuh commented Apr 13, 2017

I have no preference whether we merge this one or #22409. The changes to the Dumper class made here look a bit overcomplicated to me. On the other hand I like the split of the dumpArray() method in the Inline class.

@nicolas-grekas nicolas-grekas added this to the 3.2 milestone Apr 13, 2017
@goetas
Copy link
Contributor Author
goetas commented Apr 13, 2017

I like the latest changes in #22409, closing if flavor of it.

@goetas goetas closed this Apr 13, 2017
@goetas goetas deleted the pr-22392 branch April 25, 2017 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0