-
Notifications
You must be signed in to change notification settings - Fork 191
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
rename _toQuery() to toQuery() #760
Conversation
Are there any plans to support version 1.x with this code(or any others)? If so, please tell me the appropriate branch and I will do the same work there. |
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.
I have some small annoying asks :)
This could be a good opportunity to use toQuery
in a (new) sample.
@@ -39,8 +39,12 @@ public interface QueryVariant { | |||
|
|||
Query.Kind _queryKind(); | |||
|
|||
@Deprecated |
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.
Should we implement this one in terms of the other so we don't duplicate code? The JIT will optimize it away anyway.
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.
I've seen it being used elsewhere in code from this repository. The intention was to provide reference for internal code writers. I think it would be okay to remove it if you don't want it.
We'll backport this to 2.x, but I think 1.x is now in security patch maintenance mode only and generally with deprecations we deprecate and remove in the next major. |
Signed-off-by: channel-dante <[email protected]>
Signed-off-by: channel-dante <[email protected]>
Signed-off-by: channel-dante <[email protected]>
ad028ff
to
95972bc
Compare
@@ -36,13 +36,13 @@ This section is for maintaining a changelog for all breaking changes for the cli | |||
### Added | |||
- Added support for icu_collation_keyword type ([#725](https://github.com/opensearch-project/opensearch-java/pull/725)) | |||
- Added support for flat_object field property ([#735](https://github.com/opensearch-project/opensearch-java/pull/735)) | |||
|
|||
- Added toQuery method in Query and QueryVariant ([#760](https://github.com/opensearch-project/opensearch-java/pull/760) |
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.
Approved. If you're bored and care about my nits I would quote
all methods ;)
* rename _toQuery() to toQuery() Signed-off-by: channel-dante <[email protected]> * update CHANGELOG.md Signed-off-by: channel-dante <[email protected]> * update CHANGELOG.md Signed-off-by: channel-dante <[email protected]> --------- Signed-off-by: channel-dante <[email protected]> (cherry picked from commit b50fdb8) Signed-off-by: channel-dante <[email protected]>
* rename _toQuery() to toQuery() * update CHANGELOG.md * update CHANGELOG.md --------- (cherry picked from commit b50fdb8) Signed-off-by: channel-dante <[email protected]>
Description
Issues Resolved
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.