Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Remove broken link regression test, fix broken links #12490

Closed
wants to merge 2 commits into from
Closed

Remove broken link regression test, fix broken links #12490

wants to merge 2 commits into from

Conversation

sandeep-krishnamurthy
Copy link
Contributor

Description

  1. Remove regression check in blc (broken link checker) as these were causing false alarms and not actionable data. See discussion on dev@ thread.
  2. Fixed bunch of broken links.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • Code is well-documented:
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Remove regression broken link checks.
  • Fix bunch of broken links.

@aaronmarkham @marcoabreu

@sandeep-krishnamurthy sandeep-krishnamurthy added Doc CI pr-awaiting-review PR is waiting for code review labels Sep 9, 2018
@marcoabreu
Copy link
Contributor

Could we separate the doc fixes from the changes in the tooling?

lebeg
lebeg approved these changes Sep 10, 2018
View reviewed changes
Copy link
Contributor

@lebeg lebeg left a comment

Choose a reason for hiding this comment

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

LGTM, don't think it's a big deal of PR separation in this case, but would be good to have (we could have a crisp commit that could be cherry-picked for the release branches for example)

aaronmarkham
aaronmarkham suggested changes Sep 10, 2018
View reviewed changes
tests/nightly/broken_link_checker_test/test_broken_links.py
@@ -31,6 +31,7 @@ def prepare_link_test_result(command_output):
# Whitelisted broken links patterns
HTTP_403_REGEX = "(HTTP_403)"
HTTP_401_REGEX = "(HTTP_401)"
HTTP_409_REGEX = "(HTTP_409)"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need 301 and 303, as those are common redirect responses.
Or just do 3xx if you can so it ignores any kind of redirection.

This was referenced Sep 10, 2018
Merged
Merged
@sandeep-krishnamurthy
Copy link
Contributor Author

Thanks @lebeg and @marcoabreu - Closing this PR and created 2 minor PRs.
#12508
#12507

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@aaronmarkham aaronmarkham aaronmarkham requested changes

@lebeg lebeg lebeg approved these changes

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
CI Doc pr-awaiting-review PR is waiting for code review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sandeep-krishnamurthy @marcoabreu @lebeg @aaronmarkham