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

Add support for switching books to asciidoctor #606

Merged
merged 4 commits into from
Feb 14, 2019

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Feb 13, 2019

Adds support for switching individual books to Asciidoctor in conf.yaml.
It then cuts the glossary over to Asciidoctor. This is what the changed
docs look like:
https://github.com/elastic/built-docs-nik/commit/dfafa93c3c483bf74c2091f832c916f1ccc842ba

There are some line break changes which we can ignore. There is a change
to the HTML - in a <dd> AsciiDoc added a <p class="simpara"> tag.
Asciidoctor does not. The rendered result looks ever so slightly better
to me so I'm personally fine with it. The font size is 15px instead of
16px and the weight is 400 instead of normal.

Adds support for switching individual books to Asciidoctor in `conf.yaml`.
It then cuts the glossary over to Asciidoctor. This is what the changed
docs look like:
https://github.com/elastic/built-docs-nik/commit/dfafa93c3c483bf74c2091f832c916f1ccc842ba

There are some line break changes which we can ignore. There is a change
to the HTML - in a `<dd>` AsciiDoc added a `<p class="simpara">` tag.
Asciidoctor does not. The rendered result looks ever so slightly better
to me so I'm personally fine with it. The font size is `15px` instead of
`16px` and the weight is `400` instead of `normal`.
@nik9000 nik9000 requested a review from ddillinger February 13, 2019 15:04
@nik9000
Copy link
Member Author

nik9000 commented Feb 13, 2019

This is what AsciiDoc looks like:
asciidoc

This is what Asciidoctor looks like:
asciidoctor

@nik9000
Copy link
Member Author

nik9000 commented Feb 13, 2019

I can merge this once it gets approval from @ddillinger (perl code) and @lcawl (different rendering). That would force a fairly important change: --all will start only being supported by the docker based build (build_docs, not build_docs.pl). If you have ruby installed --all will work with build_docs.pl today, but I suspect it won't for much longer. Docker will be required.

CI is perfectly fine with this as it is running docker 100% of the time now. But folks build this locally and they'll need to be comfortable with it. The thing that jumps out at me is that --all with docker basically requires an ssh-agent and --rely_on_ssh_auth.

@nik9000
Copy link
Member Author

nik9000 commented Feb 13, 2019

❤️ @ddillinger!

@nik9000
Copy link
Member Author

nik9000 commented Feb 13, 2019

Ooh! I just realized I should update doc_build_aliases.sh as part of this too.

@lcawl
Copy link
Contributor

lcawl commented Feb 13, 2019

The asciidoctor version of the Glossary LGTM. Very exciting!

With respect to the need to use docker for --all, I suspect that it's not too common for folks to run the full build very often (particularly since it has historically taken so long). But I agree it's a good idea to double-check before we force their hand.

@lcawl lcawl self-requested a review February 13, 2019 17:11
@@ -81,6 +81,7 @@ contents:
branches: [ master ]
tags: Elastic Stack/Glossary
subject: Elastic Stack
asciidoctor: true
Copy link
Contributor

Choose a reason for hiding this comment

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

One thought: Should/could this be a list of versions (akin to "branches") instead of a boolean? If so, it would give us more wiggle room if we can't convert some of the very old releases to asciidoctor builds.

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'd be happy to do that when we figure out that we need it.

@nik9000 nik9000 merged commit 5be58ad into elastic:master Feb 14, 2019
nik9000 added a commit that referenced this pull request Feb 15, 2019
Adds support for switching individual books to Asciidoctor in `conf.yaml`.
It then cuts the glossary over to Asciidoctor. This is what the changed
docs look like:
https://github.com/elastic/built-docs-nik/commit/dfafa93c3c483bf74c2091f832c916f1ccc842ba

There are some line break changes which we can ignore. There is a change
to the HTML - in a `<dd>` AsciiDoc added a `<p class="simpara">` tag.
Asciidoctor does not. The rendered result looks ever so slightly better
to me so I'm personally fine with it. The font size is `15px` instead of
`16px` and the weight is `400` instead of `normal`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants