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

feat: add option to log warnings about redirected links #619

Closed
wants to merge 1 commit into from

Conversation

major0
Copy link

@major0 major0 commented Mar 10, 2022

closes #617

Copy link
Contributor

@cjmayo cjmayo left a comment

Choose a reason for hiding this comment

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

Can be useful to know about redirects.

Needs a test. We do have checker/test_http_redirect.py. Probably some of the tests in there can be reused, returning warning now instead of info when this is enabled?

My main comment, is that log-redirects is not the right name. The logging is a result of making a redirect a warning instead of info - and logging could still be avoided with --no-warnings.

And. of course, documentation in the man pages.

julienduchesne added a commit to grafana/terraform-provider-grafana that referenced this pull request Feb 8, 2023
I noticed that lots of links were broken in a weird way. They are marked as `latest` but redirect to v8.4 docs, which is extremely annoying
The naming scheme changed with the new docs website so I assume that it fails to find the new reference and badly redirects
I used linkchecker/linkchecker#619 to find all redirects. I should help to get that merged. Linkchecker is still active but not that PR
julienduchesne added a commit to grafana/terraform-provider-grafana that referenced this pull request Feb 8, 2023
I noticed that lots of links were broken in a weird way. They are marked as `latest` but redirect to v8.4 docs, which is extremely annoying
The naming scheme changed with the new docs website so I assume that it fails to find the new reference and badly redirects
I used linkchecker/linkchecker#619 to find all redirects. I should help to get that merged. Linkchecker is still active but not that PR
@cjmayo
Copy link
Contributor

cjmayo commented May 1, 2023

Maybe we actually make it a warning with a tag name (http-????, as https://linkchecker.github.io/linkchecker/man/linkcheckerrc.html#warnings) then it could be disabled if not required with ignorewarnings https://linkchecker.github.io/linkchecker/man/linkcheckerrc.html#url-checking-results?

As things are at the moment if it is a problem for anyone they can just stay on 10.2.1 until ready to update their config.

@ssbarnea
Copy link

I really hope to see this essential feature in. One of the most common problems is about URLs that get outdated and replaced by redirects. Now we cannot detect these redirects. What makes it worse is that sometimes these redirects are not sending to right content and instead to the external site root.

@cjmayo
Copy link
Contributor

cjmayo commented Jun 23, 2023

I think a new http- warning is the way to go (we really want to avoid more options where possible). Slightly more complex example of adding a warning: d6936ce - no need for a new method should just be replacing the add_info identified in this PR with add_warning.

@cjmayo
Copy link
Contributor

cjmayo commented Aug 26, 2023

Warning implemented in #750

@cjmayo cjmayo closed this Aug 26, 2023
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.

feature: option to report/error on redirected links
3 participants