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

Add missing items to the response body in guides/bulk.md #712

Closed
wants to merge 1 commit into from

Conversation

ap-h
Copy link

@ap-h ap-h commented Feb 10, 2024

Description

Describe what this change achieves.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

Check List

  • New functionality includes testing.
    • All tests pass
  • Linter check was successfull - yarn run lint doesn't show any errors
  • Commits are signed per the DCO using --signoff
  • Changelog was updated.

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.

@@ -147,7 +147,7 @@ client.bulk({
{ create: { _id: 2, data: { title: 'Beauty and the Beast 4', year: 2049 } } } // document already exists error
]
}).then((response) => {
response.body.forEach((item) => {
response.body.items.forEach((item) => {
Copy link
Author

@ap-h ap-h Feb 10, 2024

Choose a reason for hiding this comment

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

Is it possible items is undefined or null? According to the types it is not.

@dblock
Copy link
Member

dblock commented Feb 12, 2024

We had similar problems in guides, so we started adding working samples into https://github.com/opensearch-project/opensearch-js/tree/main/samples. Do you want to extract this code into a working sample and reference it? This way we know the code works 100%.

@ap-h
Copy link
Author

ap-h commented Feb 12, 2024

We had similar problems in guides, so we started adding working samples into https://github.com/opensearch-project/opensearch-js/tree/main/samples. Do you want to extract this code into a working sample and reference it? This way we know the code works 100%.

Thanks for the review, yep I think I can do this over the weekend, do you know any example how samples added to the guides? Just for wordings

@dblock
Copy link
Member

dblock commented Feb 12, 2024

We had similar problems in guides, so we started adding working samples into https://github.com/opensearch-project/opensearch-js/tree/main/samples. Do you want to extract this code into a working sample and reference it? This way we know the code works 100%.

Thanks for the review, yep I think I can do this over the weekend, do you know any example how samples added to the guides? Just for wordings

It's all over the place, e.g. https://github.com/opensearch-project/opensearch-js/blob/4e615f3d38bfc149fd94d3009ca824b4f6a3b862/guides/search.md#search references a sample file. I am happy with anything that links to it.

Someone will go edit all the docs, write all the samples, and make it awesome. Wink wink :)

@nhtruong
Copy link
Collaborator

nhtruong commented Mar 4, 2024

The guide has been reworked. Please check if this change is still needed anymore or if you wanna make other updates to it :)

@ap-h
Copy link
Author

ap-h commented Apr 28, 2024

Thanks @nhtruong, I don't think this is needed anymore.

@ap-h ap-h closed this Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants