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

Switches Getting Started Guide to asciidoctor builds #645

Merged
merged 1 commit into from
Feb 26, 2019

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Feb 25, 2019

This PR updates the Getting Started Guide (https://www.elastic.co/guide/en/elastic-stack-get-started/master/index.html) such that it's built via Asciidoctor.

Related to #606

This PR must not be merged until #643 is complete.

@lcawl lcawl added WIP and removed WIP labels Feb 25, 2019
@lcawl lcawl requested a review from nik9000 February 26, 2019 17:06
@lcawl
Copy link
Contributor Author

lcawl commented Feb 26, 2019

Verified that the builds against the 6.7 branch work now that #643 is merged.

@nik9000
Copy link
Member

nik9000 commented Feb 26, 2019

@debadair this one would change the anchor's id from _what_8217_s_next to _whats_next. Is that OK with you so long as no one is linking to it? I like the new text better and don't expect folks will link to the tag, but I don't fully understand any other implications of changing the anchor. I figure we change anchors from time to time when we're rewriting docs so my instinct is that it isn't too big a deal, but, like I said, I'm also keenly aware I don't know all of the implications.

@nik9000 nik9000 requested a review from debadair February 26, 2019 17:19
@lcawl
Copy link
Contributor Author

lcawl commented Feb 26, 2019

@nik9000 Actually it won't use either. I added an explicit anchor ID (whats_next) in elastic/stack-docs#228

@nik9000
Copy link
Member

nik9000 commented Feb 26, 2019

Ah!

@nik9000
Copy link
Member

nik9000 commented Feb 26, 2019

LGTM! The only difference is that Asciidoctor adds a nonbreaking space after the em dash.

@nik9000
Copy link
Member

nik9000 commented Feb 26, 2019

For reference, I ran my tests like so:

./build_docs --all --target git@github.com:elastic/built-docs-nik.git --push
<apply this branch>
./build_docs --all --target git@github.com:elastic/built-docs-nik.git --push --no_fetch
cd <a clone of built-docs-nik>
git pull
for file in $(find html/en/elastic-stack-get-started/ | grep \.html); do echo $file; docker run -it --rm --user 1000:1000 -v ~/Code/Elastic/docs:/docs_build:cached -v $(pwd):/work --security-opt seccomp=unconfined elastic/docs_build bash -c "cd /work && /docs_build/integtest/html_diff <(git show HEAD~1:$file) $file"; done

That spat out only the lines with the em dash.

@nik9000
Copy link
Member

nik9000 commented Feb 26, 2019

LGTM! The only difference is that Asciidoctor adds a nonbreaking space after the em dash.

s/nonbreaking space/zero width space/

@nik9000
Copy link
Member

nik9000 commented Feb 26, 2019

s/nonbreaking space/zero width space/

Still LGTM.

@lcawl lcawl merged commit 4bac3f3 into elastic:master Feb 26, 2019
@lcawl lcawl deleted the gs-asciidoctor branch February 26, 2019 21:42
@nik9000 nik9000 mentioned this pull request Mar 4, 2019
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.

2 participants