Skip to content

Conversation

@nik9000
Copy link
Member

@nik9000 nik9000 commented Sep 5, 2019

The source_branch attribute should be useful in resolving inter-book
links. We can get this perfect when we're running with --all because
every book already has a branch attribute. When we build with --doc
we have to guess. I've added some heuristics that I feal are likely to
guess right most of the time for most folks. I'm not too worried about
guessing wrong sometimes because, for the most part, that'll only
break interbook links and lots of folks don't check those locally
anyway. And because we have the pull request tests which run with
--all so they'll serve as an accurate backstop.

Relates to #804

The `source_branch` attribute should be useful in resolving inter-book
links. We can get this *perfect* when we're running with `--all` because
every book already *has* a branch attribute. When we build with `--doc`
we have to guess. I've added some heuristics that I feal are likely to
guess right most of the time for most folks. I'm not too worried about
guessing wrong *sometimes* because, for the most part, that'll only
break interbook links and lots of folks don't check those locally
anyway. And because we have the pull request tests which run with
`--all` so they'll serve as an accurate backstop.

Relates to elastic#804
@nik9000 nik9000 requested review from lcawl and olksdr September 5, 2019 17:53
@nik9000
Copy link
Member Author

nik9000 commented Sep 5, 2019

@olksdr I went "full perl" on this one. The only difficult part of this is the regular expression. And the testing.

@nik9000
Copy link
Member Author

nik9000 commented Sep 5, 2019

Oooh good catch past-me!

@nik9000
Copy link
Member Author

nik9000 commented Sep 5, 2019

Wrote a test for that weird case.

@lcawl
Copy link
Contributor

lcawl commented Sep 5, 2019

When I remove the source_branch attribute from elastic/stack-docs#493, the Stack Overview still builds successfully. Therefore, this LGTM

Copy link
Contributor

@olksdr olksdr left a comment

Choose a reason for hiding this comment

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

lgtm! :shipit:

@nik9000 nik9000 merged commit 49e3036 into elastic:master Sep 6, 2019
@nik9000
Copy link
Member Author

nik9000 commented Sep 6, 2019

Thanks for reviewing @olksdr!

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