-
Notifications
You must be signed in to change notification settings - Fork 339
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
Improve link extraction for Kibana link checking #2085
Conversation
- Ignore links to web content that's outside the docs - Ensure Elasticsearch Reference content is checked Fixes #2081
I don't trust my Perl skills, so there's probably a better way to do these regular expressions. I tested this locally with a copy of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'd always hoped repos could standardize on this sort of thing but I think that's not really a thing we can expect a ton of at this point.
I know that there are tests for this sort of thing off in ruby land. It'd probably be worth adding one just in case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my, perl is very much out of my wheelhouse. But I have implicit confidence in @nik9000's confidence :recursive-approval:
It's not the perl I'm confident in. It's that Greg tested it. It looks
idiomatic enough for me though.
…On Tue, Mar 16, 2021 at 5:06 PM Tyler Langlois ***@***.***> wrote:
***@***.**** approved this pull request.
Oh my, perl is very much out of my wheelhouse. But I have implicit
confidence in @nik9000 <https://github.com/nik9000>'s confidence
:recursive-approval:
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2085 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABUXIRMTCUJPTTEW5TIHOTTD7B5JANCNFSM4ZJG4T2A>
.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When tested against a version of this repo prior to when #2085 was merged, it fails with these errors: ``` rspec './integtest/spec/all_books_broken_link_detection_spec.rb[1:2:5:1]' # building all books when broken link detection is enabled when there is a link in kibana to the website outside the guide logs that all the links are ok rspec './integtest/spec/all_books_broken_link_detection_spec.rb[1:2:6:1]' # building all books when broken link detection is enabled when there is a broken Elasticsearch reference link in Kibana logs there are bad cross document links rspec './integtest/spec/all_books_broken_link_detection_spec.rb[1:2:6:2]' # building all books when broken link detection is enabled when there is a broken Elasticsearch reference link in Kibana logs the bad link ```
Fixes #2081
Related: elastic/kibana#94274, #1805