Skip to content

fix(php): Deprecation warning fix for PHP 8.1+ #3562

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

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

aykutersoy
Copy link
Contributor

@aykutersoy aykutersoy commented Aug 20, 2024

Native return types added for offsetExists, offsetGet, offsetSet, offsetUnset methods

🧭 What and Why

See: #3561

I had two option to get rid of deprecation messages, either add native types or add this #[\ReturnTypeWillChange] attribute. I opted in for native types.

🎟 JIRA Ticket: -

Changes included:

  • List changes
  • native return types added for offsetExists, offsetGet, offsetSet, offsetUnset methods

🧪 Test

for `offsetExists`, `offsetGet`, `offsetSet`, `offsetUnset` methods
@aykutersoy aykutersoy requested a review from a team as a code owner August 20, 2024 13:09
@aykutersoy aykutersoy changed the title fix(php): native return types added fix(php): Deprecation warning fix for PHP 8.1+ Aug 20, 2024
@shortcuts
Copy link
Member

Hey @aykutersoy thanks for trying out the client and providing a fix :) I've reverted some changes because they are generated files, changing the mustache one is enough

@aykutersoy
Copy link
Contributor Author

Hey @shortcuts, thanks for looking into this super fast 🙌 also adding a new commit 😆

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

looks great, thanks for the contribution!

@shortcuts shortcuts merged commit 675fc78 into algolia:main Aug 20, 2024
10 of 12 checks passed
@aykutersoy
Copy link
Contributor Author

Dang it!

@shortcuts There's one more method needs native return type
clients/algoliasearch-client-php/lib/Model/AbstractModel::jsonSerialize()
Shall I create another PR or leave it with you?

algolia-bot added a commit that referenced this pull request Aug 20, 2024
@shortcuts
Copy link
Member

Dang it!

@shortcuts There's one more method needs native return type clients/algoliasearch-client-php/lib/Model/AbstractModel::jsonSerialize() Shall I create another PR or leave it with you?

if you need it asap it would be best for you to directly contribute, otherwise we can log it on our side and tackle that later

algolia-bot added a commit to algolia/algoliasearch-client-php that referenced this pull request Aug 20, 2024
@aykutersoy
Copy link
Contributor Author

if you need it asap it would be best for you to directly contribute, otherwise we can log it on our side and tackle that later

Done! Thank you 🙌

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