Skip to content
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

FEATURE: Extend DenormalizingObjectConverter to support fromBoolean and fromInteger #2766

Merged
merged 6 commits into from
Mar 25, 2022

Conversation

bwaidelich
Copy link
Member

@bwaidelich bwaidelich commented Mar 25, 2022

...in addition to their short version (fromBool() and fromInt()).
This also adds a short documentation about Value Object property mapping.

Related: #2763

…)` and `fromInteger()`

...in addition to their short version (`fromBool()` and `fromInt()`).
This also adds a short documentation about Value Object property mapping.

Related: #2763
@bwaidelich bwaidelich requested a review from mficzel March 25, 2022 14:09
@bwaidelich bwaidelich changed the title FEATURE: Extend DenormalizingObjectConverter to support `fromBoolean(… FEATURE: Extend DenormalizingObjectConverter to support fromBoolean and fromInteger Mar 25, 2022
bwaidelich and others added 2 commits March 25, 2022 15:10
Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by reading, huge thanks for the documentation fragment

@mficzel
Copy link
Member

mficzel commented Mar 25, 2022

@bwaidelich the psalm issues should be addressed still

@bwaidelich
Copy link
Member Author

the psalm issues should be addressed still

Doh, I never really know what to do with those.. can anyone help here?

@mficzel
Copy link
Member

mficzel commented Mar 25, 2022

Looking into it. Those messages always make sense in hindsight.

@bwaidelich
Copy link
Member Author

bwaidelich commented Mar 25, 2022

Those messages always make sense in hindsight.

Well, it makes sense, because

method_exists($targetType, 'someString');

can only work if $targetType is a class-string or an object..
But I'm surprised that it didn't trip over

$targetType::fromBool($source);

before (since even PhpStorm complains about that part).

But, more importantly.. I don't know how to get rid of the message :)
I tried replacing string by string-class in the param annotation, but that bubbles up..

@mficzel mficzel force-pushed the feature/2763-extendedDenormalizingObjectConverter branch from c1c6846 to e2e67a3 Compare March 25, 2022 14:50
@mficzel
Copy link
Member

mficzel commented Mar 25, 2022

Checking for class_exists convinced psalm. It is a bit redundant but i also agree that relying on the order methods are called ist also not great.

@mficzel mficzel enabled auto-merge March 25, 2022 15:07
@mficzel mficzel merged commit 81a8483 into master Mar 25, 2022
@mficzel mficzel deleted the feature/2763-extendedDenormalizingObjectConverter branch March 25, 2022 15:12
@albe
Copy link
Member

albe commented Mar 25, 2022

Thanks a lot!

@kdambekalns
Copy link
Member

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants