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

Move out params parsing of the HttpRequest class #266

Merged
merged 2 commits into from Nov 18, 2021

Conversation

brunoocasali
Copy link
Member

This way we would gain more control about when parsing something or not that even could be
a query_params situation or a body situation.

Fixes #265

@brunoocasali
Copy link
Member Author

Before merging this @curquiza, could you please help me find every place that requires the parsing param feature?

@brunoocasali brunoocasali force-pushed the bug-fix/snake_case branch 2 times, most recently from d9f7ad4 to 08a2905 Compare November 18, 2021 03:08
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

@brunoocasali Thanks a lot for this PR! 🙏

The place where an "option" can be passed and should be transformed from snake_case into camelCase

  • index.documents method, to get all the documents
  • client.create_index method, to create an index.
  • index.update method, to update the index. Currently you can only update the primary key
  • index.search method, to apply the search
  • index.update_settings method, to update all the settings

Should NOT be applied to

  • index.update_documents and index.add_documents
  • to all the sub-settings methods, like index.update_synonyms or index.update_displayed_attributes

So the only missing point is the index.update method I guess :)

This way we would gain more control about when parse something or not that even could be
a query_params situation or a body situation.

Fixes meilisearch#265
Disable the Style/Documentation rule by default
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Thanks @brunoocasali

bors merge

@curquiza curquiza added the bug Something isn't working label Nov 18, 2021
@meili-bors
Copy link
Contributor

meili-bors bot commented Nov 18, 2021

@meili-bors meili-bors bot merged commit 34ac162 into meilisearch:main Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

⚠️ Snake case issue in v0.17.0
2 participants