Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Yaml] deprecate starting plain scalars with % characters#17809
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedFeb 15, 2016
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | symfony/symfony-docs#6269, symfony/symfony-standard#426 |
| License | MIT |
| Doc PR |
| } | ||
| if ($output &&'%' ===$output[0]) { | ||
| @trigger_error(sprintf('Not quoting a scalar starting with the "%s" indicator character is deprecated since Symfony 3.1 and will throw a ParseException in 4.0.',$output[0]),E_USER_DEPRECATED); |
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.
I don't think we need asprintf call here. Why not just using "%"?
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.
You are right. I first thought to deprecate more indicator characters. But it turned out that all other were already handled properly.
8031dc2 to1e27787Comparexabbuh commentedFeb 15, 2016
Looks like we first need to fix the dumper on lower versions. |
| $this->assertCount(1,$deprecations); | ||
| $this->assertContains('Not quoting a scalar starting with the "%" indicator character is deprecated since Symfony 3.1 and will throw a ParseException in 4.0.',$deprecations[0]); | ||
| restore_error_handler(); |
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.
This won't restore it in case of an assertion failure. This should be placed in atry/finally block (or be done before assertions)
1e27787 toba07470CompareThis PR was merged into the 2.3 branch.Discussion----------[DependencyInjection] fix dumped YAML snytax| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |see the failing tests in#17809Commits-------30388f1 [DependencyInjection] fix dumped YAML snytax
This PR was merged into the 2.8 branch.Discussion----------[Yaml] always restore the error handler in tests| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#17809 (comment)| License | MIT| Doc PR |The error handler must be restored even when assertions failed.Commits-------7631202 [Yaml] always restore the error handler in tests
fabpot commentedFeb 16, 2016
@xabbuh The other PRs have been merged now. Can you merge 2.8 into master and rebase this one? |
xabbuh commentedFeb 16, 2016
ba07470 to7e71786Compare7e71786 toe883a4cComparexabbuh commentedFeb 18, 2016
Tests finally pass. This is ready to be reviewed. ping @symfony/deciders |
fabpot commentedFeb 18, 2016
Thank you@xabbuh. |
…ers (xabbuh)This PR was merged into the 3.1-dev branch.Discussion----------[Yaml] deprecate starting plain scalars with % characters| Q | A| ------------- | ---| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets |symfony/symfony-docs#6269, symfony/symfony-standard#426| License | MIT| Doc PR |Commits-------e883a4c deprecate starting plain scalars with % characters
Since Symfony 3.1 plain scalars starting with % are deprecated.@seesymfony/symfony#17809