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

Fixes invalid links reported by linkcheck in docs #1838

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

rkratky
Copy link
Collaborator

@rkratky rkratky commented Oct 12, 2023

Fixes FR-5664

See $subj.

Copy link
Collaborator

@Chris-Peterson444 Chris-Peterson444 left a comment

Choose a reason for hiding this comment

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

So the link checker doesn't follow redirects? Good to know!

doc/reference/autoinstall-reference.rst Outdated Show resolved Hide resolved
doc/reference/autoinstall-reference.rst Outdated Show resolved Hide resolved
@rkratky
Copy link
Collaborator Author

rkratky commented Oct 13, 2023

So the link checker doesn't follow redirects? Good to know!

@Chris-Peterson444 It does. The man pages at manpages.ubuntu.com don't use HTTP redirects. AFAICT, they're JS redirects, which aren't recognized by most agents. See:

$ curl --head --location https://manpages.ubuntu.com/manpages/passwd.1.html
HTTP/2 404 
server: nginx/1.14.0 (Ubuntu)
date: Fri, 13 Oct 2023 10:04:34 GMT
content-type: text/html

@rkratky
Copy link
Collaborator Author

rkratky commented Oct 13, 2023

AFAICT, they're JS redirects

Scratch that. They pbbly use nginx rewrite.

@rkratky rkratky force-pushed the FR-5664_linkcheck-fixes branch from 6a80345 to 7347195 Compare October 13, 2023 10:40
@rkratky rkratky requested a review from ogayot October 13, 2023 10:41
Copy link
Member

@ogayot ogayot left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

Copy link
Collaborator

@dbungert dbungert left a comment

Choose a reason for hiding this comment

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

I would like you to explore linkcheck_allowed_redirects (if it works, I assume it won't) and linkcheck_ignore for the manpage problem. Or if that's not an option, we might as well bump the series to mantic at this point.

@dbungert
Copy link
Collaborator

@rkratky
Copy link
Collaborator Author

rkratky commented Oct 18, 2023

I would like you to explore linkcheck_allowed_redirects (if it works, I assume it won't) and linkcheck_ignore for the manpage problem.

Thanks for the suggestion. Looking into it.

@rkratky rkratky force-pushed the FR-5664_linkcheck-fixes branch from 7347195 to 6829f30 Compare October 18, 2023 14:52
@rkratky
Copy link
Collaborator Author

rkratky commented Oct 18, 2023

Introduced the extlinks extension to handle manpage links.

Copy link
Collaborator

@dbungert dbungert left a comment

Choose a reason for hiding this comment

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

Very happy for the linkcheck test. Please update series, RTD doesn't like extlinks at the moment, and rebase to drop the merge commit. Thanks!

doc/custom_conf.py Show resolved Hide resolved
@rkratky rkratky force-pushed the FR-5664_linkcheck-fixes branch 2 times, most recently from d8c7672 to c7c8f50 Compare October 19, 2023 12:51
(Introduced the extlinks extension for handling manpage links.)
@rkratky rkratky force-pushed the FR-5664_linkcheck-fixes branch from b50ef8c to 162a2e9 Compare October 19, 2023 13:05
@rkratky
Copy link
Collaborator Author

rkratky commented Oct 19, 2023

RTD doesn't like extlinks at the moment

Yeah, silly me :) I'm still learning how RTD and Sphinx works. I added the 'extlinks' extension where I shouldn't. I fixed it, and now it builds fine with the extension.

@dbungert dbungert merged commit 599aefd into canonical:main Oct 19, 2023
1 check passed
@rkratky rkratky deleted the FR-5664_linkcheck-fixes branch October 19, 2023 13:58
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.

4 participants