-
Notifications
You must be signed in to change notification settings - Fork 183
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
add toBuilder method on some models #766
add toBuilder method on some models #766
Conversation
85daea3
to
344d8ae
Compare
This looks good. Any way we can get some test coverage here on these other classes? I understand it's laborious (hence we should be investing in a code generator). |
@dblock I'm curious what you want to test. Can I just test whether toBuilder works (like this)? Or do you want to verify that all fields operate normally? If you want a second one, it won't be an easy task. |
I'm good with anything that just exercises the code to start. |
d897a81
to
eea2804
Compare
@dblock Can you please review again? It was a very laborious task 😅 |
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.
Looks good to me! Nit on CHANGELOG.
Thank you for doing this. I recommend investing more time in code generation so we don't have to do this by hand :)
CHANGELOG.md
Outdated
@@ -39,12 +39,15 @@ This section is for maintaining a changelog for all breaking changes for the cli | |||
- 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) | |||
- Added toBuilder method to all request model in core package & _types.query_dsl package |
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.
Add a link to the PR like other lines.
@VachaShah @reta Any ideas of how to do this kind of change better? |
I think for changes like these which span almost all files, code generation is the way to go. If we have some code which can incrementally add such methods (even if it is not a complete code generator for the client), it would be beneficial to add features like |
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.
This is great! Thank you @channel-dante for adding this along with the tests!
Agree with @VachaShah , having code generation in place would make it much simpler .... |
aebc5f5
to
df61786
Compare
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
…ypes.query_dsl Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
Signed-off-by: channel-dante <dante@channel.io>
cbceb9d
to
66c2454
Compare
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.
LGTM, thank you @channel-dante!
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-766-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a8fea24a626485173c870e220731e15482f1ab94
# Push it to GitHub
git push --set-upstream origin backport/backport-766-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x Then, create a pull request where the |
@channel-dante you should backport this manually to 2.x so we can release it with the next minor |
* add toQuery() on SearchRequest Signed-off-by: channel-dante <dante@channel.io> * add toBuilder() on all Request in org.opensearch.client.opensearch.core Signed-off-by: channel-dante <dante@channel.io> * add toBuilder() on all Builder in org.opensearch.client.opensearch._types.query_dsl Signed-off-by: channel-dante <dante@channel.io> * update CHANGELOG.md Signed-off-by: channel-dante <dante@channel.io> * apply spotlessApply Signed-off-by: channel-dante <dante@channel.io> * fix CommonTermsQuery.toBuilder() Signed-off-by: channel-dante <dante@channel.io> * fix FuzzyQuery.toBuilder() Signed-off-by: channel-dante <dante@channel.io> * fix toBuilders Signed-off-by: channel-dante <dante@channel.io> * add test for _types/query_dsl package Signed-off-by: channel-dante <dante@channel.io> * add test for core package Signed-off-by: channel-dante <dante@channel.io> * spotlessApply Signed-off-by: channel-dante <dante@channel.io> * fix test Signed-off-by: channel-dante <dante@channel.io> * revert ModelTestCase Signed-off-by: channel-dante <dante@channel.io> * revert ModelTestCase Signed-off-by: channel-dante <dante@channel.io> * add pr link on CHANGELOG.md Signed-off-by: channel-dante <dante@channel.io> * add pr link on CHANGELOG.md Signed-off-by: channel-dante <dante@channel.io> * apply spotless Signed-off-by: channel-dante <dante@channel.io> --------- Signed-off-by: channel-dante <dante@channel.io> (cherry picked from commit a8fea24) Signed-off-by: channel-dante <dante@channel.io>
* add toBuilder method on some models (#766) * add toQuery() on SearchRequest Signed-off-by: channel-dante <dante@channel.io> * add toBuilder() on all Request in org.opensearch.client.opensearch.core Signed-off-by: channel-dante <dante@channel.io> * add toBuilder() on all Builder in org.opensearch.client.opensearch._types.query_dsl Signed-off-by: channel-dante <dante@channel.io> * update CHANGELOG.md Signed-off-by: channel-dante <dante@channel.io> * apply spotlessApply Signed-off-by: channel-dante <dante@channel.io> * fix CommonTermsQuery.toBuilder() Signed-off-by: channel-dante <dante@channel.io> * fix FuzzyQuery.toBuilder() Signed-off-by: channel-dante <dante@channel.io> * fix toBuilders Signed-off-by: channel-dante <dante@channel.io> * add test for _types/query_dsl package Signed-off-by: channel-dante <dante@channel.io> * add test for core package Signed-off-by: channel-dante <dante@channel.io> * spotlessApply Signed-off-by: channel-dante <dante@channel.io> * fix test Signed-off-by: channel-dante <dante@channel.io> * revert ModelTestCase Signed-off-by: channel-dante <dante@channel.io> * revert ModelTestCase Signed-off-by: channel-dante <dante@channel.io> * add pr link on CHANGELOG.md Signed-off-by: channel-dante <dante@channel.io> * add pr link on CHANGELOG.md Signed-off-by: channel-dante <dante@channel.io> * apply spotless Signed-off-by: channel-dante <dante@channel.io> --------- Signed-off-by: channel-dante <dante@channel.io> (cherry picked from commit a8fea24) Signed-off-by: channel-dante <dante@channel.io> * removed not related test code Signed-off-by: channel-dante <dante@channel.io> --------- Signed-off-by: channel-dante <dante@channel.io>
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.