-
Notifications
You must be signed in to change notification settings - Fork 190
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 support to parse sub-aggregations from filter/nested aggregations #234
Add support to parse sub-aggregations from filter/nested aggregations #234
Conversation
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 wonder whether we'd be better off separating the classes across tests. In this change Product
serves multiple tests, so eventually it becomes a mess, doesn't it?
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.
We really need a USER_GUIDE similar to https://github.com/opensearch-project/opensearch-js/blob/main/USER_GUIDE.md here. Would you mind starting that as part of this PR with just documenting aggregations?
5e7136b
to
7013ddc
Compare
Sorry, was busy with work. I have added a USER_GUIDE.md with most of the basic operations (including aggregations). Please review and approve the PR now. |
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 great, thanks! Let's add a link to USER_GUIDE from README as well. Doesn't have to block this PR.
I have added the link to USER_GUIDE in the README. |
@dblock Can we please merge this PR now? I really need it for a project. Thanks. |
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.
The CHANGELOG verifier is complaining, need to add a line to that. Sorry about that ;(
Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
…ject#241) Signed-off-by: Meetesh Kumawat<kmeetesh@gmail.com> Signed-off-by: meetesh <kmeetesh@gmail.com> Signed-off-by: Meetesh Kumawat<kmeetesh@gmail.com> Signed-off-by: meetesh <kmeetesh@gmail.com> Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
…search-project#240) Signed-off-by: Andriy Redko <andriy.redko@aiven.io> Signed-off-by: Andriy Redko <andriy.redko@aiven.io> Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
* Updates changelog for dependabot PRs Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> * Adding dependabot label for workflow Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io> Signed-off-by: Andriy Redko <andriy.redko@aiven.io> Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
* Update literature Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> * Removing pr template and updating language Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> Co-authored-by: Daniel (dB.) Doubrovkine <dblock@amazon.com> Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
* Bump grgit-gradle from 4.0.1 to 5.0.0 Bumps [grgit-gradle](https://github.com/ajoberstar/grgit) from 4.0.1 to 5.0.0. - [Release notes](https://github.com/ajoberstar/grgit/releases) - [Commits](ajoberstar/grgit@4.0.1...5.0.0) --- updated-dependencies: - dependency-name: org.ajoberstar.grgit:grgit-gradle dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Update changelog Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com> Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
deefafd
to
4bd4a43
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.
Below ;)
Btw, if you want to make your commit look nicer after I squash it on merge, you can always squash and force push on the client. This can be a little tricky, I usually create a new branch off main, merge the change and force push it over, something like this (haven't actually tried):
git checkout main
git checkout -b subaggregations-squashed
git merge subaggregations --squash
git comit -s -m ....
git push origin -f subaggregations-squashed:subaggregations
For small changes on top of my PRs I usually amend/force push. I even have an alias called git commend
for it.
Signed-off-by: Abhinav Nath <abhinavnath@ymail.com>
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.
Thank you @abhinav-nath!
Thank you. Could you please add a "HACKTOBERFEST-ACCEPTED" label to this PR? Would be great for my dev profile 😊 |
Can someone do it please? |
Done @abhinav-nath |
Thank you @dblock 🙏🏼 |
Signed-off-by: Abhinav Nath abhinavnath@ymail.com
Description
Add support to parse sub-aggregations from filter/nested aggregations.
Issues Resolved
Link
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.