Skip to content

do linkchecks for changelog lint #1722

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

Closed
RonnyPfannschmidt opened this issue Jul 13, 2016 · 24 comments · Fixed by #5614
Closed

do linkchecks for changelog lint #1722

RonnyPfannschmidt opened this issue Jul 13, 2016 · 24 comments · Fixed by #5614
Labels
good first issue easy issue that is friendly to new contributor type: infrastructure improvement to development/releases/CI structure

Comments

@RonnyPfannschmidt
Copy link
Member

#1721 showed that we don't check those and mistakes happen, #1720 related

@nicoddemus
Copy link
Member

Does anyone have a suggestion on how to do this? I took a look at rst-lint we use and it does not have that option.

@The-Compiler
Copy link
Member

LinkChecker maybe? Hopefully it also runs locally over the generated HTML files.

If we do this with Travis we probably shouldn't do it on every push though.

@nicoddemus
Copy link
Member

Hmm sounds good. Why do you say we shouldn't do it on every push? So people's PRs are not blocked if some unrelated link is broken?

Python 3 is not yet supported.

Sad panda is sad.

@The-Compiler
Copy link
Member

No, because it's not very nice to do dozens (hundreds?) of HTTP request with every push 😉 I guess we could use Travis' cron feature?

@RonnyPfannschmidt
Copy link
Member Author

should we make it part of the pre-release process?

@nicoddemus
Copy link
Member

No, because it's not very nice to do dozens (hundreds?) of HTTP request with every push 😉

Heh, right. 😝

should we make it part of the pre-release process?

Good idea. A simple environment on tox that should be part of the release process sounds good, executed locally.

@nicoddemus nicoddemus added the type: docs documentation improvement, missing or needing clarification label Jul 13, 2016
@adamtheturtle
Copy link

In case it saves you a few minutes, https://github.com/jenca-cloud/jenca-authentication/blob/master/.travis.yml#L16 is a .travis.yml which only installs and runs linkchecker on Python 3 builds.

@nicoddemus
Copy link
Member

Thanks @adamtheturtle!

@The-Compiler
Copy link
Member

Seems like Sphinx also has a linkcheck builder built-in.

@nicoddemus nicoddemus added type: infrastructure improvement to development/releases/CI structure and removed type: docs documentation improvement, missing or needing clarification labels Sep 28, 2017
@Zac-HD Zac-HD added the good first issue easy issue that is friendly to new contributor label Dec 5, 2018
@Zac-HD
Copy link
Member

Zac-HD commented Dec 5, 2018

An easy contribution would be to run the Sphinx linkcheck builder from release.py, after the fix_formatting() step.

@mandarvaze
Copy link

Anyone working on this ? I would like to work on this.

I'm a first time contributor, and may require a bit of help, especially how to test the change etc. (Relevant code snippet and possible solution already provided by @Zac-HD - Thanks.)

@blueyed
Copy link
Contributor

blueyed commented Jun 3, 2019

Anyone working on this ? I would like to work on this.

I don't think anyone is working on it.. go ahead :)

@mandarvaze
Copy link

@Zac-HD

I'm assuming once broken links are reported, someone needs to fix those (There seem to be a lot, right now when I was testing my changes)

I'm thinking of the following :

diff --git a/tox.ini b/tox.ini
index 7e5b182f6..129dabb96 100644
--- a/tox.ini
+++ b/tox.ini
@@ -66,6 +66,7 @@ changedir = doc/en
 deps = -r{toxinidir}/doc/en/requirements.txt

 commands =
+    sphinx-build -b linkcheck . _build
     sphinx-build -W -b html . _build

 [testenv:doctesting]

This will be called when building documentation locally, and provides opportunity to fix the broken links (if they choose)

Downsides :

  1. Local build will take additional time (unnecessary ?)
  2. Since I don't know how often is the tox -e docs command used, I'm worried that as @The-Compiler said, we might be generating too many (unnecessary) requests.

Please suggest.

@blueyed
Copy link
Contributor

blueyed commented Jun 3, 2019

I think a separate toxenv would be good, which could inject args into the existing one using a tox env (see related hacks there already), e.g.

setenv =
  checklinks: _TOX_PYTEST_SPHINX_ARGS=-b linkcheck

Then running tox -e docs-checklinks would set the env var, that would be used in the command.

Not sure about if that should run on CI always - quite unnecessary, especially with non-doc changes.

@Zac-HD
Copy link
Member

Zac-HD commented Jun 3, 2019

Probably not a good idea to run too often, as a linkcheck is inherently flaky (due to transient network issues, timeouts, etc).

Ideally we'd have a new tox env that just does the linkcheck, which can be run before making a release. And add this to the release checklist!

@mandarvaze
Copy link

Ideally we'd have a new tox env that just does the linkcheck, which can be run before making a release.

Understood. I can do what @blueyed suggested.

And add this to the release checklist!

Do you mean update HOWTORELEASE.rst ?
or
call it from release.py as you suggested above ?

@nicoddemus
Copy link
Member

@mandarvaze just call it from release.py should be fine. 👍

steffenschroeder added a commit to steffenschroeder/pytest that referenced this issue Jul 13, 2019
Following pytest-dev#1722 this will check all links in documentation.
@steffenschroeder
Copy link
Contributor

started to working on this during EuroPython2019 sprint. (with the help of @The-Compiler)
Will open 2 PRs:

  1. for the broken links (sphinx-build's linkcheck only reports one error at a time so it takes some time)
  2. for adding tox and and release.py

@steffenschroeder
Copy link
Contributor

Is https://docs.pytest.org/en/latest/adopt.html (April 2015 is "adopt pytest month") still needed? It is still in the repo and contains a broken link (to google forms). Can this be deleted? If not, what can I with the link?

@The-Compiler
Copy link
Member

@steffenschroeder I'd keep it around - it's nice to know it was a thing that happened, and we should probably do that again some day! Which of the links is broken? The two form links I see there seem to work fine. I'd probably just remove the link and maybe strike through the text or so.

@steffenschroeder
Copy link
Contributor

@The-Compiler the Partner projects, sign up here! is broken an can probably be removed (or strike through).
The assert, markers and distributed links could be changed to proper locations. Will do so.

steffenschroeder added a commit to steffenschroeder/pytest that referenced this issue Jul 15, 2019
Following pytest-dev#1722 this will check all links in documentation.
steffenschroeder added a commit to steffenschroeder/pytest that referenced this issue Jul 15, 2019
Following pytest-dev#1722 this will check all links in documentation.
@The-Compiler
Copy link
Member

@steffenschroeder Odd, that link seems to be okay - it leads to a The form "Request a helper for your open source project, for "adopt pytest month"" is no longer accepting responses.-page, but that's not a 404 or anything, so I wonder why linkcheck complains about it. Either way, seems fine to remove the text.

steffenschroeder added a commit to steffenschroeder/pytest that referenced this issue Jul 16, 2019
Following pytest-dev#1722 this will check all links in documentation.
@steffenschroeder
Copy link
Contributor

steffenschroeder commented Jul 16, 2019

@The-Compiler although the link works in the browser, checklinks will report it as an error. So I have 2 options:

  1. remove the link
  2. add it to a whitelist as false positive

For this particular case, I removed it.

steffenschroeder added a commit to steffenschroeder/pytest that referenced this issue Jul 16, 2019
Following pytest-dev#1722 this will check all links in documentation.
@steffenschroeder
Copy link
Contributor

#5614 will add link checking for all links in the documentation.
The check will only be done locally during the release process (release.py)

#5613 will correct all broken links the linkchecker found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue easy issue that is friendly to new contributor type: infrastructure improvement to development/releases/CI structure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants