Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Oct 14, 2019

Relates #47510

@tlrx tlrx added >docs General docs changes :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features v8.0.0 v7.5.0 labels Oct 14, 2019
@tlrx tlrx requested review from dnhatn and martijnvg October 14, 2019 09:32
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/CCR)

@tlrx tlrx requested a review from jrodewig October 14, 2019 15:39

//////////////////////////

[source,console]
Copy link
Contributor

Choose a reason for hiding this comment

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

In this section, we typically use literals instead of an actual snippet:

POST /_ccr/auto_follow/<auto_follow_pattern_name>/pause

You may want to move the preceding test setup lower but no big deal.

--------------------------------------------------
POST /_ccr/auto_follow/<auto_follow_pattern_name>/resume
--------------------------------------------------
// TEST[s/<auto_follow_pattern_name>/my_auto_follow_pattern/]
Copy link
Contributor

Choose a reason for hiding this comment

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

We typically use literals here:
POST /_ccr/auto_follow/<auto_follow_pattern_name>/resume

Copy link
Member Author

Choose a reason for hiding this comment

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

I made it like this to mirror what is done in docs/reference/ccr/apis/follow/post-pause-follow.asciidoc but maybe it's wrong there too?

Copy link
Contributor

Choose a reason for hiding this comment

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

The format here is based on the API reference template:
https://github.com/elastic/docs/blame/master/shared/api-ref-ex.asciidoc#L39

It's likely that docs/reference/ccr/apis/follow/post-pause-follow.asciidoc followed a previous iteration of that template.

If you'd like to leave this as-is for consistency, I think that's okay. I don't feel strongly about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I forgot about this template. I adapted the doc to follow the template and not add more debt.

Copy link
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.

Looks good overall.

I left a recommended needed changes in these comments:
https://github.com/elastic/elasticsearch/pull/47985/files#r334560561
https://github.com/elastic/elasticsearch/pull/47985/files#r334558730

I also left a few other non-blocking suggestions.

Copy link
Member

@dnhatn dnhatn 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 Tanguy.

@tlrx tlrx merged commit 58ca9ee into elastic:master Oct 15, 2019
@tlrx tlrx deleted the add-pause-resume-docs branch October 15, 2019 06:24
tlrx added a commit that referenced this pull request Oct 15, 2019
@tlrx
Copy link
Member Author

tlrx commented Oct 15, 2019

Thanks @dnhatn and @jrodewig !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Indexing/CCR Issues around the Cross Cluster State Replication features >docs General docs changes v7.5.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants