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

Re-integrate Sphinx's linkcheck into CI configuration #12932

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ Kyle Altendorf
Lawrence Mitchell
Lee Kamentsky
Lev Maximov
Ladislav Chvastas
ladislav987 marked this conversation as resolved.
Show resolved Hide resolved
Levon Saldamli
Lewis Cowles
Llandy Riveron Del Risco
Expand Down
14 changes: 11 additions & 3 deletions doc/en/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,18 @@
linkcheck_ignore = [
"https://blogs.msdn.microsoft.com/bharry/2017/06/28/testing-in-a-cloud-delivery-cadence/",
"http://pythontesting.net/framework/pytest-introduction/",
r"https://github.com/pytest-dev/pytest/issues/\d+",
r"https://github.com/pytest-dev/pytest/pull/\d+",
r"https://pypi\.org/project/pytest.*",
Copy link
Member

Choose a reason for hiding this comment

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

This matches any project page starting with pytest. Like pytesty. Why? I don't think I've seen any problems with this host in linkcheck in the past.

r"https://github\.com/sponsors/.*",
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary? I don't remember such URLs not working in other projects.

Copy link
Author

Choose a reason for hiding this comment

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

    r"https://github.com/pytest-dev/pytest/issues/\d+",
    r"https://github.com/pytest-dev/pytest/pull/\d+",

this is from the original version. But you're right, this doesn't trigger any issues with link.

Copy link
Author

Choose a reason for hiding this comment

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

Without r"https://github\.com/sponsors/.*", checking will trigger issues:
WARNING: broken link: https://github.com/sponsors/xxx (429 Client Error: Too Many Requests for url: https://github.com/xxx)

It seems to be related to missing credentials or tokens for GitHub but I'm not 100% sure.

Copy link
Member

Choose a reason for hiding this comment

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

With xxx specifically, I suppose? How about with real existing usernames?

r"https://github\.com/pytest-dev/pytest/issues/.*",
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be more accurate. Such a wildcard would also silence obviously broken URLs like https://github.com/pytest-dev/pytest/pull/zombie that should produce errors otherwise.

r"https://github\.com/pytest-dev/pytest/pull/.*",
]
linkcheck_workers = 5

linkcheck_workers = 20
linkcheck_timeout = 30
linkcheck_retries = 2
linkcheck_anchors = False
linkcheck_rate_limit_timeout = 2.0
linkcheck_delay = 2.0
Comment on lines +150 to +155
Copy link
Member

Choose a reason for hiding this comment

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

What are the defaults of these?


# -- Options for HTML output ----------------------------------------------------------
# https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-output
Expand Down
3 changes: 2 additions & 1 deletion doc/en/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
pluggy>=1.5.0
pygments-pytest>=2.3.0
sphinx-removed-in>=0.2.0
sphinx>=7
sphinx>=8.1.3
sphinx_issues
Copy link
Member

Choose a reason for hiding this comment

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

sphinx-issues is already listed below

Suggested change
sphinx_issues

Copy link
Author

Choose a reason for hiding this comment

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

FIxed

sphinxcontrib-trio
sphinxcontrib-svg2pdfconverter
furo
Expand Down
7 changes: 4 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,10 @@ description =
basepython = python3
usedevelop = True
changedir = doc/en
deps = -r{toxinidir}/doc/en/requirements.txt
deps =
-r{toxinidir}/doc/en/requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

could you keep such formatting changes out?

Copy link
Author

Choose a reason for hiding this comment

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

FIxed

commands =
sphinx-build -W -q --keep-going -b linkcheck . _build
sphinx-build -W -q --keep-going -b linkcheck -j auto . _build
setenv =
# Sphinx is not clean of this warning.
PYTHONWARNDEFAULTENCODING=
Expand All @@ -143,7 +144,7 @@ passenv =
deps =
PyYAML
regendoc>=0.8.1
sphinx
sphinx>=8.1.3
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this should instead just reference the requirements file or the base section value...

allowlist_externals =
make
commands =
Expand Down
Loading