Skip to content
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

[Yaml] Fixed invalid Parser behavior #27898

Merged
merged 1 commit into from
Feb 6, 2019
Merged

Conversation

guiguiboy
Copy link
Contributor

@guiguiboy guiguiboy commented Jul 8, 2018

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #27874
License MIT
Doc PR NA

This fixes #27874
I'm not sure about the update in composer.json though. It seems a good idea because I was able to run composer update without the zip extension. If required, I'll remove it.

fabpot
fabpot previously requested changes Jul 12, 2018
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

/cc @xabbuh

composer.json Outdated Show resolved Hide resolved
stof
stof previously requested changes Jul 12, 2018
src/Symfony/Component/Yaml/Parser.php Outdated Show resolved Hide resolved
@fabpot
Copy link
Member

fabpot commented Sep 4, 2018

@xabbuh Can you review this one one more time please?

@stof
Copy link
Member

stof commented Sep 4, 2018

As this introduce a new deprecation, it should go in master, not in 4.1. We don't introduce deprecations in patch releases.

@guiguiboy
Copy link
Contributor Author

Hello, I missed your message. I'll rebase shortly.

@guiguiboy guiguiboy changed the base branch from 4.1 to master October 2, 2018 19:45
@xabbuh
Copy link
Member

xabbuh commented Oct 10, 2018

@guiguiboy I will look into the changes in more depth shortly. Thanks so far. 👍 Can you in the meantime please document the deprecation in the upgrade files and in the changelog file of the Yaml component?

@guiguiboy
Copy link
Contributor Author

I updated the files supposing it will go in version 5.0.0. If required, I can change back to 4.2.0.

@chalasr chalasr modified the milestones: 4.1, next Nov 1, 2018
src/Symfony/Component/Yaml/Parser.php Outdated Show resolved Hide resolved
src/Symfony/Component/Yaml/Parser.php Outdated Show resolved Hide resolved
UPGRADE-5.0.md Outdated Show resolved Hide resolved
src/Symfony/Component/Yaml/CHANGELOG.md Outdated Show resolved Hide resolved
src/Symfony/Component/Yaml/CHANGELOG.md Outdated Show resolved Hide resolved
@guiguiboy guiguiboy force-pushed the ticket_27874 branch 2 times, most recently from 069fa71 to 532cce8 Compare December 28, 2018 15:17
Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

with a minor tweak to the deprecation message

src/Symfony/Component/Yaml/Parser.php Outdated Show resolved Hide resolved
@xabbuh xabbuh dismissed stale reviews from stof and fabpot February 6, 2019 08:39

changes have been addressed

@xabbuh
Copy link
Member

xabbuh commented Feb 6, 2019

Thank you @guiguiboy.

@xabbuh xabbuh merged commit 7bf8381 into symfony:master Feb 6, 2019
xabbuh added a commit that referenced this pull request Feb 6, 2019
This PR was merged into the 4.3-dev branch.

Discussion
----------

[Yaml] Fixed invalid Parser behavior

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27874
| License       | MIT
| Doc PR        | NA

This fixes #27874
I'm not sure about the update in composer.json though. It seems a good idea because I was able to run composer update without the zip extension. If required, I'll remove it.

Commits
-------

7bf8381 Added deprecation notice when mapping keys are found in multi-line blocks
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@fabpot fabpot mentioned this pull request May 9, 2019
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.

[Yaml] parses misplaced keys in strings
7 participants