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

[DOCS] Replace nested open block for Asciidoctor migration #41168

Merged
merged 2 commits into from
Apr 22, 2019
Merged

[DOCS] Replace nested open block for Asciidoctor migration #41168

merged 2 commits into from
Apr 22, 2019

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Apr 12, 2019

Per asciidoctor/asciidoctor#1121, Asciidoctor does not currently allow nested open blocks. This removes the nested open block from the Index modules page and replaces it with a table.

This prevents the following error:
INFO:build_docs:asciidoctor: ERROR: illegal block content outside of partintro block

Relates to #41128

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs

@jrodewig jrodewig requested review from nik9000 and lcawl April 12, 2019 20:24
@nik9000
Copy link
Member

nik9000 commented Apr 12, 2019

If the table renders well I'm happy with it. I know we usually use definition lists because tables tend to come out poorly when we try to use them for this sort of thing.

@jrodewig
Copy link
Contributor Author

jrodewig commented Apr 12, 2019

Thanks for the feedback @nik9000. Here's a screenshot from my local build:

index-modules

I agree that tables generally aren't preferred, but I couldn't figure out a way to nest a definition list and still stay Asciidoctor valid. Please feel free to leave a suggestion or point me the right way if I'm overlooking something though.

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

That looks fine to me!

@lcawl
Copy link
Contributor

lcawl commented Apr 15, 2019

Another possible solution that seems to work for me is this:

Accepts:
false::: (default) Don't check for corruption when opening a shard.
checksum::: Check for physical corruption.
true::: Check for both physical and logical corruption. This is much more
expensive in terms of CPU and memory usage.
+
WARNING: Expert only. Checking shards may take a lot of time on large indices.

@jrodewig
Copy link
Contributor Author

jrodewig commented Apr 15, 2019

Thanks for the solve @lcawl. Pushed c5b9aa2 to reformat the table to a definition list.

I feel much better about this without a table.

Updated screenshot:
table-to-list

Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

LGTM and builds successfully

@jrodewig jrodewig merged commit 9476a78 into elastic:master Apr 22, 2019
@jrodewig jrodewig deleted the fix-asciidoctor-errors-index-modules branch April 22, 2019 12:39
jrodewig added a commit that referenced this pull request Apr 22, 2019
* [DOCS] Fix nested open blocks for Asciidoctor migration

* [DOCS] Reformat table to definitions
jrodewig added a commit that referenced this pull request Apr 22, 2019
* [DOCS] Fix nested open blocks for Asciidoctor migration

* [DOCS] Reformat table to definitions
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
…1168)

* [DOCS] Fix nested open blocks for Asciidoctor migration

* [DOCS] Reformat table to definitions
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.

5 participants