-
Notifications
You must be signed in to change notification settings - Fork 37
Update jsonSerialize methods to stop deprecations errors in php 8.1 #34
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
Thanks for the pull request!! To ensure quality review, Couchbase employs a code review system based on Gerrit to manage the workflow of changes in addition to tracking our contributor agreements. To get this change in and collaborate in code review, please register on Gerrit and accept our CLA. The easiest way to do this is to follow the link below, sign in with your GitHub account and then follow through the steps provided on that page to sign an 'Individual' agreement: http://review.couchbase.org/#/settings/new-agreement. Keep in mind that the emails we are seeing on the commits are: Note: Please contact us if you have any issues registering with Gerrit! If you have not signed our CLA within 7 days, the Pull Request will be automatically closed. |
Thanks @srjlewis, could you sign our CLA, please? After that we can accept your contribution. |
@avsej I have already signed the CLA, If I follow the link https://review.couchbase.org/settings/new-agreement it say "Agreement already submitted." |
Hey @srjlewis , Can you confirm that the email mentioned by the bot above ( Cheers, Brett |
Hi @brett19 I have linked that email account and I have seen the buildbot run. |
Hey @srjlewis, You're right, my apologizes. It looks like something has gone wrong with our bot! I'll get it fix asap and get this merged. Cheers, Brett |
Hi @brett19 Is any of the build testing done against PHP 8.1 yet? I have done the simple tests on my dev machine, I know it is not a complicated fix, but just wondering about your testing on php 8.1. |
Hi @srjlewis, thanks for contribution. The support for 8.1 will be added into our test pipeline shortly, and next release will have all code necessary to build the extension with PHP 8.1. If you want your patch to be accepted with your attribution, please review your account on our Gerrit system, and make sure the emails configured there match the emails in the commits here in PR, otherwise I will just do the fix myself when I will be configuring PHP 8.1 on our CI. |
Hi @avsej, Thanks for the update, My email addresses are already linked to Gerrit to the best of my knowledge. |
I have changed the ZEND_BEGIN_ARG_INFO_EX call to ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX on any methods that are used by the PHP JsonSerializable interface, to stop deprecation notices in PHP 8.1
Hope this helps.