8000 [Yaml] Deprecate tags using colon by GuilhemN · Pull Request #22913 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Yaml] Deprecate tags using colon #22913

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

[Yaml] Deprecate tags using colon #22913

wants to merge 2 commits into from

Conversation

GuilhemN
Copy link
Contributor
@GuilhemN GuilhemN commented May 25, 2017 •
edited
Loading
Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Using a colon in a tag doesn't look like yaml and causes trouble (see #22878), so I propose to just deprecate these tags in favor of more consistent tags.

- !php/const:PHP_INT_MAX
- !php/object:O:30:"Symfony\Component\Yaml\Tests\A":1:{s:1:"a";s:3:"foo";}

would become

- !php/const PHP_INT_MAX
- !php/object O:30:"Symfony\Component\Yaml\Tests\A":1:{s:1:"a";s:3:"foo";}

@xabbuh
Copy link
Member
xabbuh commented May 25, 2017

👍 (we will probably still need some special handling for the !php/const tag though as referring to class constants still requires us to use colons inside a mapping key)

@xabbuh xabbuh added this to the 3.4 milestone May 25, 2017
@GuilhemN
Copy link
Contributor Author

The handling won't be dedicated, the value will just need to be quoted:

!php/const "Foo::BAR": quz

@GuilhemN
Copy link
Contributor Author

PR broken for now, will fix it later.

@GuilhemN GuilhemN changed the title [Yaml] Deprecate tags using colon [WIP][Yaml] Depre 8000 cate tags using colon May 29, 2017
@teohhanhui teohhanhui mentioned this pull request Jun 28, 2017
Merged
stof
stof requested changes Jun 28, 2017
View reviewed changes
UPGRADE-3.4.md Outdated
```yml
!php/const PHP_INT_MAX
```
>>>>>>> [Yaml] Deprecate tags using colon
Copy link
Member

Choose a reason for hiding this comment

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

this line must be removed. You messed some conflict resolution

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed

@GuilhemN GuilhemN changed the title [WIP][Yaml] Deprecate tags using colon [Yaml] Deprecate tags using colon Jun 29, 2017
@GuilhemN
Copy link
Contributor Author

Rebased on 3.4, it should be ok now.

@fabpot
Copy link
Member
fabpot commented Jul 6, 2017

@xabbuh Can you check this one please?

@teohhanhui teohhanhui mentioned this pull request Jul 18, 2017
Merged
@GuilhemN GuilhemN force-pushed the tagswithcomma branch 2 times, most recently from 84bd287 to 8ee9609 Compare July 24, 2017 08:31
@GuilhemN
Copy link
Contributor Author

The build failure is because this is not yet in master.

@chalasr
Copy link
Member
chalasr commented Jul 31, 2017

Needs rebase

@GuilhemN
Copy link
Contributor Author
GuilhemN commented Aug 1, 2017

@chalasr rebased.

@xabbuh xabbuh mentioned this pull request Aug 4, 2017
Merged
xabbuh
xabbuh approved these changes Aug 4, 2017
View reviewed changes
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.

just a few minor comments

src/Symfony/Component/Yaml/CHANGELOG.md Outdated
@@ -4,6 +4,12 @@ CHANGELOG
3.4.0
-----

* Deprecated the tag`!php/object:` tag which will be replaced by the
Copy link
Member

Choose a reason for hiding this comment

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

the "tag" in front of !php/object: should be removed

src/Symfony/Component/Yaml/CHANGELOG.md Outdated
* Deprecated the tag`!php/object:` tag which will be replaced by the
`!php/object` tag (without the colon) in 4.0.

* Deprecated the tag`!php/const:` tag which will be replaced by the
Copy link
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

the "tag" in front of !php/const: should be removed

src/Symfony/Component/Yaml/Inline.php Outdated
@@ -653,6 +667,20 @@ private static function evaluateScalar($scalar, $flags, $references = array())
}

return;

Copy link
Member

Choose a reason for hiding this comment

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

no need for this blank line

@GuilhemN
Copy link
Contributor Author
GuilhemN commented Aug 4, 2017

@xabbuh fixed

@xabbuh
Copy link
Member
xabbuh commented Aug 4, 2017

Thank you @GuilhemN.

xabbuh added a commit that referenced this pull request Aug 4, 2017
@xabbuh
minor #22913 [Yaml] Deprecate tags using colon (GuilhemN)
baa1e7f 
This PR was squashed before being merged into the 3.4 branch (closes #22913).

Discussion
----------

[Yaml] Deprecate tags using colon

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Using a colon in a tag doesn't look like yaml and causes trouble (see #22878), so I propose to just deprecate these tags in favor of more consistent tags.

```yml
- !php/const:PHP_INT_MAX
- !php/object:O:30:"Symfony\Component\Yaml\Tests\A":1:{s:1:"a";s:3:"foo";}
```
would become
```yml
- !php/const PHP_INT_MAX
- !php/object O:30:"Symfony\Component\Yaml\Tests\A":1:{s:1:"a";s:3:"foo";}
```

Commits
-------

9815af3 [Yaml] Deprecate tags using colon
@xabbuh xabbuh closed this Aug 4, 2017
@GuilhemN GuilhemN deleted the tagswithcomma branch August 4, 2017 15:03
@xabbuh xabbuh mentioned this pull request Aug 4, 2017
Merged
@teohhanhui teohhanhui mentioned this pull request Apr 12, 2018
Merged
@rhift rhift mentioned this pull request Jan 2, 2020
Closed
@nicolas-grekas nicolas-grekas mentioned this pull request Jun 24, 2022
Merged
fabpot added a commit that referenced this pull request Jun 25, 2022
@fabpot
minor #46768 [Yaml] Remove legacy parsing rule (nicolas-grekas)
69c5859 
This PR was merged into the 6.2 branch.

Discussion
----------

[Yaml] Remove legacy parsing rule

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

As far as I understood correctly, this part of the regexp has been introduced in #22878 to support a syntax that has been deprecated in #22913 and removed in 4.0.

Commits
-------

5d7f9f2 [Yaml] Remove legacy parsing rule
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@stof stof stof approved these changes

@xabbuh xabbuh xabbuh approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Deprecation Status: Needs Review Status: Needs Work Yaml
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
6 participants
@GuilhemN @xabbuh @fabpot @chalasr @stof @carsonbot
0