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
Deprecate camel case attributes #377
Conversation
bf46328
to
0fb81e4
Compare
Hi, @jmks thank you so much for your contribution! It is amazing! Let's answer your questions:
I think we can use the same logic for every method that has a
I just sent a recommendation because of the Meilisearch name, but it is ok for me.
Yeah, I didn't have much context of the code when I wrote it in the first place, but today I don't know any use case for that handler. But to be honest this code will be removed in a couple of releases after the next one, because now we will have the deprecation warning :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmks your PR is approved in any case, but I would like to have the warning in every possible use case, so if you could do this change it would be awesome! :D
This message is sent automatically Thanks again for contributing to Meilisearch ❤️ |
Hi, @jmks do you still have time to finish this PR? Otherwise I can do it anyway :) |
Sorry, didn't mean to sit on this for so long. I did start working on it again today and will update the PR in the next day or so. |
All the specs use snake case attributes now and dropping these did not affect test coverage.
18154a1
to
0c63f47
Compare
Hi! I've updated the all callers of
I'm not sure about that. My impression of this was that in the Ruby code, this is standardizing on snake_case (as per Ruby idiom) but when sending to the Meilisearch server (or receiving responses), the attributes will still be camelCase. |
I continued your work in branch #470 Thank you very much for your work, and sorry for taking that long to make this move. |
Pull Request
Related issue
Fixes #320
What does this PR do?
When calling
index#search
with non-snake case attributes (e.g.camelCase
orweirdMixed_Case
) a warning is emitted (to stderr). Here's an example:I have a few questions:
The issue mentioned
sending the search attributes
. Do you want this deprecation warning for just index#search or all methods (i.e. any caller ofUtils.transform_attributes
)?Currently this just handles index#search but should be easy adapt for all callers.
I wrote something I would like to see but I'm open to suggestions.
Utils.transform_attributes
handles an Array argument.I ran the test suite and did not find an example of an array passed to this function. Do you know when this could be called with an Array argument?
.code-samples.meilisearch.yml
that needed changing (again, only for index#search).However, I did update the specs.
Cheers!
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!