-
Notifications
You must be signed in to change notification settings - Fork 6
Update for compatibility with symfony/serializer 7 #46
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
Conversation
I just ran into the same incompatibility and thanks for the effort, just my two cents: shouldn't this update also include updating/setting up the used |
Hi @theteknocat and @finwe Thank you for bringing this issue to my attention! Sorry I overlooked it. As finwe explained, instead of just updating the content in the More generally, update/upgrade tasks for this library are divided into two parts:
I am going to open a separate pull request that does both of this changes, and I'll let you test if this works for your case? Thanks a lot again for your interest in this library 🙂 |
So here it is: #47 By upgrading Jane and letting it re-generate the SDK, it has automatically added a compat layer, so this new version will work in environments with recent or older versions of the serializer. See for example the class harvest-php-api/generated/Normalizer/BillableRatesNormalizer.php Lines 25 to 249 in 106493c
@theteknocat can you please test the branch associated with #47 in your environment to check that it works well? If you do not mind, I am going to merge #47 and I'll let you close this one? |
@xavierlacot @finwe thanks for your responses and for flagging the correct way to do this. I had no idea how to wonk with the Jane php api, so this is definitely the better solution and having a compatibility layer that works with both versions is ideal. Of course this now creates a conflict with the commit I made to the manually updated generated code, but that can be reverted easily enough. I'll try to find some time in the next few days to test this in my environment and see if it works. |
@xavierlacot after updating my codebase to use v7.0, everything works as expected. Thanks so much for taking care of that! |
Updated the function signatures of normalize and denormalize to match those defined by the interfaces being called for. Also updated the denormalize method to use an updated argument name.
Resolves #45.