Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Serializer] Trigger deprecation when could not parse date with default format#58966
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
base:7.4
Are you sure you want to change the base?
Conversation
@nicolas-grekas Could you have a look? |
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.
Could you update the Serializer'sCHANGELOG.md file and theUPGRADE-7.3.md file as well?
src/Symfony/Component/Serializer/Normalizer/DateTimeNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
….phpCo-authored-by: Mathias Arlaud <mathias.arlaud@gmail.com>
Looks like the tests must be updated to not trigger the deprecation. |
| @@ -126,6 +126,8 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a | |||
| if (false !== $object = $type::createFromFormat($defaultDateTimeFormat, $data, $timezone)) { | |||
| return $object; | |||
| } | |||
| trigger_deprecation('symfony/serializer', '7.3', \sprintf('A "%s" will be thrown when a date could not be parsed using the default format "%s".', NotNormalizableValueException::class, $defaultDateTimeFormat)); | |||
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 think we should trigger a deprecation here only if the fallback manages to instantiate it. The case where the fallback throws an exception (already turned into a NotNormalizableValueException below) should not trigger a deprecation.
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.
As I understand, in 8.0, the fallback should be removed, 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.
yes. And that's my point: if the fallback also fails, you already have the same behavior than what would happen in 8.0 and we should not trigger a deprecation in that case as the behavior will not change. Try submittingfoobar as value to reproduce that case.
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 have rechecked and now realise that fallback should not be removed in8.0. Fallback should still be used whendatetime_format is set tonull.
66a1ede to663eec5Compare@mtarld I have done with your comments. |
Relates to#43329 (comment)