-
Notifications
You must be signed in to change notification settings - Fork 72
chore: add link-checking command #551
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
base: main
Are you sure you want to change the base?
Conversation
…kage-guide into ci/add-linkchecking
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.
Pull Request Overview
This PR adds an automated link-checking step for the Sphinx documentation and updates a number of broken or redirected URLs throughout the guide.
- Introduces a new
docs-linkcheck
nox session with linkcheck parameters and output directory - Configures Sphinx’s linkcheck in
conf.py
and documents the session inCONTRIBUTING.md
- Fixes several external links (Hatch, setuptools-scm, peer-reviewed badge) and updates translations accordingly
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tutorials/add-readme.md | Update peer-reviewed badge link to software-submission |
tests/run-tests.md | Update Hatch repository URL to pypa/hatch |
package-structure-code/python-package-versions.md | Update setuptools-scm repo URL |
noxfile.py | Add docs-linkcheck session and linkcheck constants |
conf.py | Configure Sphinx linkcheck ignore rules |
CONTRIBUTING.md | Document the new docs-linkcheck nox session |
documentation/repository-files/readme-file-best-practices.md | Update peer-reviewed badge link in best practices guide |
locales/ja/LC_MESSAGES/tests.po | Update Hatch URL in Japanese translation |
locales/ja/LC_MESSAGES/package-structure-code.po | Update setuptools-scm URL in Japanese translation |
locales/ja/LC_MESSAGES/documentation.po | Update peer-reviewed badge URL in Japanese translation |
locales/es/LC_MESSAGES/tests.po | Update Hatch URL in Spanish translation |
locales/es/LC_MESSAGES/package-structure-code.po | Update setuptools-scm URL in Spanish translation |
locales/es/LC_MESSAGES/documentation.po | Update peer-reviewed badge URL in Spanish translation |
Comments suppressed due to low confidence (3)
package-structure-code/python-package-versions.md:226
- [nitpick] The link text uses an underscore but the project name has a hyphen. Consider updating
Setuptools_scm
tosetuptools-scm
for consistency with the repository name.
[`Setuptools_scm`](https://github.com/pypa/setuptools-scm/) is an
noxfile.py:78
- This new
docs-linkcheck
session isn't covered by any automated tests. Consider adding a test or CI job to ensure the linkcheck session runs as expected.
@nox.session(name="docs-linkcheck")
CONTRIBUTING.md:193
- [nitpick] You might also want to mention this new session in the project README or main documentation index so contributors know about the
docs-linkcheck
command.
nox -e docs-linkcheck
""" | ||
Check that links are well-formed and point to something that exists. | ||
""" | ||
session.install("-e", ".") |
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.
Consider creating the LINKCHECK_OUTPUT_DIR before running Sphinx to avoid errors if the directory does not exist (e.g., LINKCHECK_OUTPUT_DIR.mkdir(parents=True, exist_ok=True)).
session.install("-e", ".") | |
session.install("-e", ".") | |
# Ensure LINKCHECK_OUTPUT_DIR exists | |
LINKCHECK_OUTPUT_DIR.mkdir(parents=True, exist_ok=True) |
Copilot uses AI. Check for mistakes.
Reading through the packaging guide, as part of working on pyOpenSci/pyosPackage#56, I noticed that there are a lot of links to external resources in the packaging guide.
Enough that I think it'd be worth adding some automation to check the validity of those links.
@agriyakhetarpal recently taught me (in jameslamb/pydistcheck#293 (comment)) about 2 forms of this that are really useful and do complementary things:
sphinx -b linkcheck
: check links in Sphinx docs (docs on how to do this)lychee
: check all other links in the project (docs on how to do this)This PR proposes introducing a
nox
session that can be used to run the Sphinx link checks. As of this PR, running this from the root of the repo:Generates a summary like this:
46 remaining broken or redirecting URLs (click me)
It also fixes a few link issues that were caught by this:
hatch
has moved to thepypa
GitHub orgsetuptools-scm
is now spelled with a-
not a_
These were all successfully redirecting today, but using the direct targets will make page loads a little faster and reduce the risk of those links being broken if those redirections are eventually removed in the future.
Notes for Reviewers
Thanks for your time and consideration. If this is annoying please close it and sorry for the noise.
Why not fix all the links?
I just didn't want this to be too large, especially since I'm new to this project.
If maintainers are receptive to it, I'd be happy to create an issue documenting the remaining broken links, so folks can contribute fixes for them.
Why not also add
lychee
?Again just trying to keep this small and focused. If maintainers are receptive, I'd be happy to put up a follow-up PR proposing adding that. Here's what it can look like: jameslamb/pydistcheck#312
Should this run in CI?
It would be helpful, but you might find that link checks break so often that they're disruptive to have in CI that runs on every commit to every PR. In my experience, for every 1 "this was permanently deleted" type of error, you'll experience like 25 of the form "temporary network disruption", "server down for maintenance", etc.
A once-a-week scheduled job and a README badge making the status visible is probably enough. Up to yall though, that's definitely bigger than this PR and depends on your interest. I'd be happy to create an issue or follow-up PR if you'd like.
How I tested this