Skip to content

Conversation

@Nicksqain
Copy link
Contributor

Description

  • Create advanced index actions guide

Using the doc: Opensearch rust

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.

Copy link
Collaborator

@Xtansia Xtansia left a comment

Choose a reason for hiding this comment

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

Your guide is still referencing and using Ruby, it has not been updated to Rust.

I think it would be beneficial for you to create an accompanying opensearch/examples/advanced_index_actions.rs example file that combines all the snippets together to ensure the code is valid, compiles and can be run successfully.

@Xtansia Xtansia added documentation Improvements or additions to documentation skip-changelog Skip changelog verification CCI College Contributor Initiative labels Apr 10, 2023
@Nicksqain Nicksqain requested a review from Xtansia April 14, 2023 14:31
@dblock
Copy link
Member

dblock commented Apr 14, 2023

  • Let's link this new guide from USER_GUIDES?
  • Link checker is complaining.

I think we already are discussing how to run these samples in CI, separate issue but thought I'd mention it.

@codecov-commenter
Copy link

codecov-commenter commented Apr 14, 2023

Codecov Report

Merging #147 (70b587d) into main (dd166f0) will increase coverage by 0.31%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #147      +/-   ##
==========================================
+ Coverage   65.07%   65.38%   +0.31%     
==========================================
  Files         312      314       +2     
  Lines       51715    52182     +467     
==========================================
+ Hits        33652    34119     +467     
  Misses      18063    18063              
Flag Coverage Δ
integration 65.38% <ø> (+0.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 34 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Alexei Karikov <[email protected]>
@Nicksqain
Copy link
Contributor Author

Nicksqain commented Apr 16, 2023

@dblock

  • Check if this is how you want to add the link?
    The way I see it is that there will be links to all the guides in the file. And in the new guide I can add a link to .rs, where you can see how to import and use the code.

  • And is it my fault in the CI tests?

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I like it. Link checker needs to pass.

Consider linking to the documentation concepts like "force merge" when they are mentioned.

@@ -0,0 +1,113 @@
# Advanced Index Actions

In this guide, we will look at some advanced index actions that are not covered in the [Index Lifecycle](index_lifecycle.md) guide.
Copy link
Member

Choose a reason for hiding this comment

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

That file doesn't exist yet, so the link checker breaks. You can combine all the guides in 1 PR or make them in order they link to each-other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dblock Can we wait for the index lifecycle to merge first and then re-run the link checker?

Signed-off-by: Alexei Karikov <[email protected]>
Signed-off-by: Alexei Karikov <[email protected]>
Signed-off-by: Alexei Karikov <[email protected]>
@Nicksqain Nicksqain requested review from Xtansia and dblock April 19, 2023 18:25
@Xtansia Xtansia merged commit 9d7b91f into opensearch-project:main Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CCI College Contributor Initiative documentation Improvements or additions to documentation skip-changelog Skip changelog verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Guide] Advanced Index Actions

4 participants