Skip to content

[DOCS] Collapse contents in cluster reroute docs#54851

Merged
jrodewig merged 2 commits intoelastic:masterfrom
patelvp:docs/cluster-reroute
Apr 9, 2020
Merged

[DOCS] Collapse contents in cluster reroute docs#54851
jrodewig merged 2 commits intoelastic:masterfrom
patelvp:docs/cluster-reroute

Conversation

@patelvp
Copy link
Copy Markdown
Contributor

@patelvp patelvp commented Apr 6, 2020

Summary
Collapsible docs for cluster reroute document as mentioned in #54667
Uses changes in elastic/docs#1786

Tests
Ran ./gradlew -p docs check tests pass.

@patelvp patelvp changed the title Collapse contents in cluster reroute docs [DOCS] Collapse contents in cluster reroute docs Apr 6, 2020
@henningandersen henningandersen added the >docs General docs changes label Apr 7, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-docs (>docs)

@jrodewig
Copy link
Copy Markdown
Contributor

jrodewig commented Apr 7, 2020

@elasticmachine test this please

@jrodewig jrodewig self-requested a review April 7, 2020 13:55
Comment on lines +99 to +100
=====
====
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These block delimiters are unneeded and preventing the docs build from working properly.

Suggested change
=====
====

Comment on lines +73 to +75
.Properties of `metric`
[%collapsible%open]
======
Copy link
Copy Markdown
Contributor

@jrodewig jrodewig Apr 7, 2020

Choose a reason for hiding this comment

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

Because these are query parameters, they're not objects per se. It's more accurate to describe them as `options.

I'm not sure we need the collapsible section here as these won't be nested more than one level deep.

Suggested change
.Properties of `metric`
[%collapsible%open]
======
.Options for `metric`
[%collapsible%open]
======

@@ -58,28 +58,30 @@ query parameter, which will attempt a single retry round for these shards.
==== {api-query-parms-title}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
==== {api-query-parms-title}
[role="child_attributes"]
==== {api-query-parms-title}

`POST /_cluster/reroute`


[role="child_attributes"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This attribute should be included in the sections containing the collapsible sections.

Suggested change
[role="child_attributes"]

Comment on lines +141 to +142
=====
====
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
=====
====

@@ -109,10 +114,13 @@ include::{docdir}/rest-api/common-parms.asciidoc[tag=timeoutparms]
(Required, object) Defines the commands to perform. Supported commands are:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

GitHub won't allow me to add a comment, but [role="child_attributes"] needs to added directly above ==== {api-request-body-title} or [[cluster-reroute-api-request-body]] around line 110.

@@ -109,10 +114,13 @@ include::{docdir}/rest-api/common-parms.asciidoc[tag=timeoutparms]
(Required, object) Defines the commands to perform. Supported commands are:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
(Required, object) Defines the commands to perform. Supported commands are:
(Required, array of objects) Defines the commands to perform. Supported commands are:

for index name and shard number, and `node` to allocate the shard to. Takes
<<modules-cluster,allocation deciders>> into account.
--
======
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would move this delimiter after the definition for allocate_empty_primary. allocate_stale_primary and allocate_empty_primary are properties for a commands object.

Copy link
Copy Markdown
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

Hi @patelvp

Thanks for submitting this PR. I left some comments that contain needed changes before we can move forward.

I'll take another look once those are addressed.

@jrodewig
Copy link
Copy Markdown
Contributor

jrodewig commented Apr 9, 2020

@elasticmachine test this please

Copy link
Copy Markdown
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @patelvp. I'll merge and backport once the CI tests pass.

@jrodewig jrodewig merged commit 7003ac4 into elastic:master Apr 9, 2020
@jrodewig
Copy link
Copy Markdown
Contributor

jrodewig commented Apr 9, 2020

Backport commits

master 7003ac4
7.x 51cb0c5

@patelvp
Copy link
Copy Markdown
Contributor Author

patelvp commented Apr 9, 2020

Thanks @jrodewig. This is my first open source pull request. Many more to go.

@jrodewig
Copy link
Copy Markdown
Contributor

jrodewig commented Apr 9, 2020

Thanks for making us your first contribution, @patelvp!

@jakelandis jakelandis removed the v8.0.0 label Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants