Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Serializer] Remove last deprecated/obsolete paths#31771
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
src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| if (class_exists(MimeTypes::class)) { | ||
| $mimeTypeGuesser = MimeTypes::getDefault(); | ||
| }else { | ||
| @trigger_error(sprintf('Passing null to "%s()" without symfony/mime installed is deprecated since Symfony 4.3, install symfony/mime.',__METHOD__),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.
this should be an exception now?
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.
That's what I thought too (see second commit), but the data uri normalizer used to fallback toapplication/octet-stream if no default mime type guesser can be instantiated. So neither was the http-foundation nor the mime type component a strict dependency to use it.
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 notice is wrong in the first place and should be removed from 4.3 then, right?
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.
Indeed. See#31923
src/Symfony/Component/Serializer/Normalizer/DataUriNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| if (class_exists(MimeTypes::class)) { | ||
| $mimeTypeGuesser = MimeTypes::getDefault(); | ||
| }else { | ||
| @trigger_error(sprintf('Passing null to "%s()" without symfony/mime installed is deprecated since Symfony 4.3, install symfony/mime.',__METHOD__),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.
this notice is wrong in the first place and should be removed from 4.3 then, right?
src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| * @param object $object | ||
| * @param string|null $format | ||
| * @param array $context | ||
| * @param object $object |
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.
we can add the type and remove the annotation actually - PHP7.2 allows it
| * @param string|null $format | ||
| */ | ||
| protectedfunctioncreateChildContext(array$parentContext,$attribute/*, ?string $format */) | ||
| protectedfunctioncreateChildContext(array$parentContext,string$attribute, ?string$format):array |
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.
should have null as default, otherwise that's a hard BC break
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 method is internal. That should be fine, right?
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.
@internal is not transitive
once all are correctly annotated on 4.4, sure :)
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.
fixed in2b8e441
…guesser is optional) (ogizanagi)This PR was merged into the 4.3 branch.Discussion----------[Serializer] Fix DataUriNormalizer deprecation (MIME type guesser is optional)| Q | A| ------------- | ---| Branch? | 4.3 <!-- see below -->| Bug fix? | yes| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | N/ARelates to#31771 (comment) :The deprecation isn't fundamentally wrong, but if none of the Mime nor HttpFoundation components are installed, the DataUriNormalizer can be used, and the MIME type used will always be `'application/octet-stream'`https://github.com/symfony/symfony/blob/9691519ca46511005bc701ca93c2973923952034/src/Symfony/Component/Serializer/Normalizer/DataUriNormalizer.php#L39-L46https://github.com/symfony/symfony/blob/9691519ca46511005bc701ca93c2973923952034/src/Symfony/Component/Serializer/Normalizer/DataUriNormalizer.php#L135-L139So this completes the deprecation message, as well as allowing `null` again when no default MIME type guesser is available at all.Commits-------2740bd1 [Serializer] Fix DataUriNormalizer deprecation (MIME type guesser is optional)
nicolas-grekas commentedJun 8, 2019
Thank you@ogizanagi. |
…anagi)This PR was merged into the 5.0-dev branch.Discussion----------[Serializer] Remove last deprecated/obsolete paths| Q | A| ------------- | ---| Branch? | master <!-- see below -->| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets |#28316,#28709,#31030,#27020,#29896,16f8a13#r201060750 <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | N/A <!-- required for new features -->This should fix the last deprecations & obsolete code paths for the Serializer component.Commits-------c703b35 [Serializer] Remove last deprecated/obsolete paths
Uh oh!
There was an error while loading.Please reload this page.
This should fix the last deprecations & obsolete code paths for the Serializer component.