-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Serializer] Add COLLECT_EXTRA_ATTRIBUTES_ERRORS and full deserialization path
#46654
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
base: 8.1
Are you sure you want to change the base?
[Serializer] Add COLLECT_EXTRA_ATTRIBUTES_ERRORS and full deserialization path
#46654
Conversation
689dfd9 to
f8d653d
Compare
|
Would ideally need #45861 first. |
e285d19 to
7b8a73a
Compare
|
Seems I cannot reproduce the failing tests in local in 8.2? Some help will be appreciated for this :) |
3328a3f to
dac569d
Compare
|
Hey! I think @mtarld has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
dac569d to
58fc21e
Compare
|
Any news for this? |
58fc21e to
a202da2
Compare
daebc6d to
adf10e3
Compare
| * returned. Otherwise, the concatenation of the two paths is returned, | ||
| * separated by a dot ("."). | ||
| */ | ||
| public static function append(string $basePath, string $subPath): string |
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 PropertyPathInterface by adding a new @method phpdoc attribute?
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 this is a static method, I don't think it makes sense to add in in the PropertyPathInterface.
src/Symfony/Component/PropertyAccess/Tests/PropertyPathTest.php
Outdated
Show resolved
Hide resolved
adf10e3 to
ac2c423
Compare
|
I think I am done with the changes here. 👍 |
b01db33 to
389804b
Compare
src/Symfony/Component/Serializer/Exception/PartialDenormalizationException.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Serializer/Exception/PartialDenormalizationException.php
Outdated
Show resolved
Hide resolved
389804b to
536fb94
Compare
|
Done 👍 |
mtarld
left a comment
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.
LGTM then, thank you @NorthBlue333!
src/Symfony/Component/Serializer/Exception/PartialDenormalizationException.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Serializer/Exception/PartialDenormalizationException.php
Outdated
Show resolved
Hide resolved
| public function getErrors(): array | ||
| { | ||
| return $this->errors; | ||
| return $this->getNotNormalizableValueErrors(); |
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 should throw a runtime deprecation
can you remind me why we deprecated the method?
deprecating always comes with a cost so it needs a good enough reason
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.
Well this was because it might be used. Should I completely remove it / throw an exception instead of deprecating it?
536fb94 to
2cda4a9
Compare
2cda4a9 to
2e6a44f
Compare
|
@NorthBlue333 may be close PR since no movements? |
COLLECT_EXTRA_ATTRIBUTES_ERRORS and full deserialization path
The
PartialDenormalizationExceptionis used as unexpected extra attributes are, IMO, part of the denormalization exception.This enables executing: