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.

Sign up for GitHub

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 19 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d60d253
update python version to 3.12
Nov 1, 2024
024917a
added name
Nov 1, 2024
1cb2f70
latest version of sphinx
Nov 1, 2024
9e5a6a1
latest version of sphinx
Nov 1, 2024
e7b64e9
new configuration
Nov 1, 2024
029fb15
updated command + alternative
Nov 1, 2024
b95e56f
Merge branch 'main' into chvastas/bring-back-link-checks
Pierre-Sassoulas Nov 1, 2024
9777e1b
removed dead code
Nov 2, 2024
3f2e0c9
Merge remote-tracking branch 'origin/chvastas/bring-back-link-checks'…
Nov 2, 2024
0bb4166
edited linkcheck_ignore
Nov 2, 2024
3ea3e2f
deleted duplicated deps
Nov 2, 2024
22f5296
Merge branch 'main' into chvastas/bring-back-link-checks
ladislav987 Nov 7, 2024
683121f
Merge branch 'main' into chvastas/bring-back-link-checks
Pierre-Sassoulas Nov 10, 2024
7128c3d
Merge branch 'main' into chvastas/bring-back-link-checks
ladislav987 Nov 11, 2024
cabba81
fix misplaced name
Nov 11, 2024
b4655f9
fix duplicity
Nov 11, 2024
3e48d69
fix formatting
Nov 11, 2024
dfaada7
Merge remote-tracking branch 'origin/chvastas/bring-back-link-checks'…
Nov 11, 2024
423818b
Merge branch 'main' into chvastas/bring-back-link-checks
Pierre-Sassoulas Jan 11, 2025
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
View file
Open in desktop
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
17 changes: 16 additions & 1 deletion doc/en/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,23 @@
"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://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://pypi\.org/project/pytest.*",
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.

Yes in theory the dot should be escaped in a regex, as it's not a dot, it's "any character", but it works and is simpler imo. (like in the two links above). Or we can make the two links above consistent :)

Suggested change
r"https://github\.com/sponsors/.*",
r"https://pypi\.org/project/pytest.*",
r"https://github\.com/pytest-dev/pytest/issues/.*",
r"https://github.com/sponsors/.*",
r"https://pypi.org/project/pytest.*",

I think we should modify the existing regex line 144 /145 so they accept link to a comment, or remove them entirely in favor of the simpler one you added.

Copy link
Author

Choose a reason for hiding this comment

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

I simplified it to:

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://pypi\.org/project/pytest.*",
    r"https://github\.com/sponsors/.*",
    r"https://github\.com/pytest-dev/pytest/issues/.*",
    r"https://github\.com/pytest-dev/pytest/pull/.*",
]

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.

]
linkcheck_workers = 5

# # Exclude documents that are known to cause issues
# linkcheck_exclude_documents = [
# 'old_versions/*',
# 'deprecated_features/*',
# ]

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 +151 to +156
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
View file
Open in desktop
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
12 changes: 9 additions & 3 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,18 @@ 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

sphinx>=8.1.3
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
commands =
sphinx-build -W -q --keep-going -b linkcheck . _build
sphinx-build -W -q --keep-going -b linkcheck -j auto . _build
; /bin/sh -c "sphinx-build -W -q --keep-going -b linkcheck -j auto . _build > linkcheck_output.txt 2>&1"

setenv =
# Sphinx is not clean of this warning.
PYTHONWARNDEFAULTENCODING=
;allowlist_externals =
; /bin/sh

[testenv:regen]
description =
Expand All @@ -143,7 +149,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