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] 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 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?
[Serializer] Add COLLECT_EXTRA_ATTRIBUTES_ERRORS and full deserialization path#46654
Uh oh!
There was an error while loading.Please reload this page.
Conversation
689dfd9
tof8d653d
CompareWould ideally need#45861 first. |
e285d19
to7b8a73a
CompareNorthBlue333 commentedJun 13, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Seems I cannot reproduce the failing tests in local in 8.2? Some help will be appreciated for this :) |
3328a3f
todac569d
Comparecarsonbot commentedJun 13, 2022
Hey! I think@mtarld has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
dac569d
to58fc21e
CompareAny news for this? |
58fc21e
toa202da2
Comparedaebc6d
toadf10e3
CompareUh oh!
There was an error while loading.Please reload this page.
* 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 thePropertyPathInterface
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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
adf10e3
toac2c423
CompareI think I am done with the changes here. 👍 |
b01db33
to389804b
Comparesrc/Symfony/Component/Serializer/Exception/PartialDenormalizationException.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Serializer/Exception/PartialDenormalizationException.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
389804b
to536fb94
CompareDone 👍 |
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 OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Serializer/Exception/PartialDenormalizationException.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
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?
Uh oh!
There was an error while loading.Please reload this page.
536fb94
to2cda4a9
Compare2cda4a9
to2e6a44f
Compare
Uh oh!
There was an error while loading.Please reload this page.
The
PartialDenormalizationException
is used as unexpected extra attributes are, IMO, part of the denormalization exception.This enables executing: