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

Deserializes.php broke if the type is an DateTimeInterface due to only checking for classes and not interfaces #732

Merged
merged 4 commits into from
Jun 18, 2024

Conversation

KhorneHoly
Copy link

Deserialization broke while creating the DTO from the SellingPartnerApi\Seller\ReportsV20210630\Responses\Report class, since the dates all are of the type DateTimeInterface.

…DateTimeInterface, so that the serialization process triggered an exception.
@KhorneHoly KhorneHoly changed the title Deserializes.php broke if the type is an DateTimeInterface due to only checken for classes and not interfaces Deserializes.php broke if the type is an DateTimeInterface due to only checking for classes and not interfaces Jun 13, 2024
@jlevers
Copy link
Owner

jlevers commented Jun 14, 2024

Couple minor comments, but looks good! Thanks for catching this. The reason for the Zulu format comment is that we changed the format in #699, too.

@KhorneHoly
Copy link
Author

KhorneHoly commented Jun 17, 2024

Couple minor comments, but looks good! Thanks for catching this. The reason for the Zulu format comment is that we changed the format in #699, too.

Hi, thanks for the reply. You said something about minor comments, but I wasn't able to find anything. Am I missing something? Hit me up if there's need to change.

src/Traits/Deserializes.php Outdated Show resolved Hide resolved
src/Traits/Deserializes.php Outdated Show resolved Hide resolved
@jlevers
Copy link
Owner

jlevers commented Jun 17, 2024

Whoops, sorry...I hadn't clicked "Submit review." Should be visible now.

@KhorneHoly
Copy link
Author

Whoops, sorry...I hadn't clicked "Submit review." Should be visible now.

No worries, I've pushed a new commit with the respective changes. Also updated the exception message.

@jlevers jlevers merged commit fa96840 into jlevers:main Jun 18, 2024
2 checks passed
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.

2 participants