Skip to content

feat(php): Model cleaning #137

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 7 commits into from
Feb 15, 2022
Merged

feat(php): Model cleaning #137

merged 7 commits into from
Feb 15, 2022

Conversation

damcou
Copy link
Contributor

@damcou damcou commented Feb 11, 2022

🧭 What and Why

🎟 JIRA Ticket: APIC-304

Changes included:

  • Removed models generation
  • Update php doc to use arrays as in the current PHP client.

🧪 Test

@damcou damcou requested a review from millotp February 11, 2022 14:56
Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

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

Can you explain why you don't think model are useful ?
Is there a way to make them useful by typing parameters ?
Is it possible to only type the response and not the params ?

@damcou
Copy link
Contributor Author

damcou commented Feb 15, 2022

Can you explain why you don't think model are useful ? Is there a way to make them useful by typing parameters ? Is it possible to only type the response and not the params ?

Passing arrays as parameters make sense in PHP, it is how it's done right now in the current client https://github.com/algolia/algoliasearch-client-php/blob/master/src/SearchIndex.php and globally PHP developers are more used to pass arrays in methods.
Maybe it would be possible to find a system that let them use either objects or arrays (I'll create a ticket for this) but I don't think this is mandatory for the beta.

@millotp
Copy link
Collaborator

millotp commented Feb 15, 2022

Is it not possible to wrap the response or convert it to the correct type ? It helps so much during development

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.

3 participants