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

[Rename] Update README.markdown #517

Closed
wants to merge 2 commits into from

Conversation

harold-wang
Copy link
Contributor

Description

[Describe what this change achieves]
Change Elastic to OpenSearch in UpdateREADME.markdown

Issues Resolved

[List any issues this PR will resolve]
#221

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

Signed-off-by: Harold Wang harowang@amazon.com
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.

Signed-off-by: Harold Wang <harowang@amazon.com>
Signed-off-by: Harold Wang <harowang@amazon.com>
@odfe-release-bot
Copy link

✅   DCO Check Passed 1bf92c4

@harold-wang harold-wang self-assigned this Apr 9, 2021
@harold-wang harold-wang added the documentation Improvements or additions to documentation label Apr 9, 2021
@odfe-release-bot
Copy link

✅   Gradle Wrapper Validation success f3b96a6

@harold-wang
Copy link
Contributor Author

@tlfeng @setiah appreciate if you can help review this one by the end of today, thank you.

@odfe-release-bot
Copy link

✅   DCO Check Passed f3b96a6

@odfe-release-bot
Copy link

✅   Gradle Precommit success f3b96a6

Copy link
Member

@CEHENKLE CEHENKLE left a comment

Choose a reason for hiding this comment

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

We'll hold merge on this until documentation is ready.

yet-to-be-created doc pages can break the [Elastic docs
build](https://github.com/elastic/docs#building-documentation).
yet-to-be-created doc pages can break the [OpenSearch docs
build](https://github.com/opensearch/docs#building-documentation).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good point, because I didn't aware that there are Elasticsearch clients using the links in the API specification for their documents.
Currently no documents are generated by the URLs in OpenSearch docs build process. For now, OpenSearch only supports the official Java client, and there is an open issue to track the clients for supporting other languages.
In my opinion, the sentence in **IMPORANT:** can be removed, because it probably will be invalid for a long time.

@adnapibar
Copy link
Contributor

I don't think this PR is related to the issue it mentions. Can you please create an issue (unless there is already one, seems like there are two related ones #70 & #107) indicating to replace all links to the existing docs URL with OpenSearch docs URL. And when the website is ready, let's do a sweeping change across the repository to replace all links in one single PR.

@CEHENKLE
Copy link
Member

I don't think this PR is related to the issue it mentions. Can you please create an issue (unless there is already one, seems like there are two related ones #70 & #107) indicating to replace all links to the existing docs URL with OpenSearch docs URL. And when the website is ready, let's do a sweeping change across the repository to replace all links in one single PR.

Hey -- that's on me. I was considering changing just this particular file, but then decided to hold off. You're right, though, that we should have a metaissue to track all of the URL changes related to docs (if we don't already). Thanks, Rabi!

@@ -43,12 +43,12 @@ Example for the ["Create Index"](https://www.elastic.co/guide/en/elasticsearch/r
The specification contains:

* The _name_ of the API (`indices.create`), which usually corresponds to the client calls
* Link to the documentation at the <http://elastic.co> website.
* Link to the documentation at the <http://opensearch.org> website.

**IMPORANT:** This should be a _live_ link. Several downstream ES clients use
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove "ES" from here and just say "clients"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dblo

Should we remove "ES" from here and just say "clients"?

Yes, thanks.

@harold-wang
Copy link
Contributor Author

This PR becomes unreachable (https://support.github.com/ticket/personal/0/1103734) this morning, surprised to see that this PR link became accessible after closing, a replacement PR #530 was created, let keep #517 closed.

@adnapibar appreciate if you can ping me or ask me to close it before your close the PRs that I opened next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants