-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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: 7.3
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 👍 |
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
The
PartialDenormalizationException
is used as unexpected extra attributes are, IMO, part of the denormalization exception.This enables executing: