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

Valid DOI redirected links trigger warnings. #1344

Closed
charleskawczynski opened this issue Jun 18, 2020 · 8 comments · Fixed by #1345
Closed

Valid DOI redirected links trigger warnings. #1344

charleskawczynski opened this issue Jun 18, 2020 · 8 comments · Fixed by #1345

Comments

@charleskawczynski
Copy link

I'm seeing lots of doc warnings regarding redirected DOI links. For example

┌ Warning: linkcheck 'https://doi.org/10.1016/0169-8095%2894%2900090-Z' status: 302, redirects to https://linkinghub.elsevier.com/retrieve/pii/016980959400090Z.
└ @ Documenter.DocChecks ~/.julia/packages/Documenter/PLD7m/src/DocChecks.jl:223

Both the first and second links work. It doesn't seem like this should be a warning.

@simonbyrne
Copy link
Contributor

simonbyrne commented Jun 18, 2020

It's especially confusing since in this case since they trigger an @warn, but don't actually cause a linkcheck failure:

if location !== nothing
@warn "linkcheck '$(link.url)' status: $(status), redirects to $(location)."
else
@warn "linkcheck '$(link.url)' status: $(status)."
end

whereas other errors that do trigger linkcheck failures sometimes trigger an @warn:
push!(doc.internal.errors, :linkcheck)
@warn "invalid result returned by $cmd:" result

and sometimes an @error:
push!(doc.internal.errors, :linkcheck)
@error "linkcheck '$(link.url)' status: $(status)."

Would it be possible to make them so that any linkcheck failures always trigger an @error?

@simonbyrne
Copy link
Contributor

In this specific case, I don't think 302 errors should trigger a warning, since it is meant to be used as a temporary redirect:
https://en.wikipedia.org/wiki/HTTP_302

simonbyrne added a commit to simonbyrne/Documenter.jl that referenced this issue Jun 18, 2020
@mortenpi mortenpi added this to the 0.25.0 milestone Jun 21, 2020
@mortenpi
Copy link
Member

Would it be possible to make them so that any linkcheck failures always trigger an @error?

An argument one could make is that e.g. with 301 redirects the links are still technically working, so we shouldn't error on them.

@simonbyrne
Copy link
Contributor

I meant any failure which does push!(doc.internal.errors, :linkcheck)

@charleskawczynski
Copy link
Author

An argument one could make is that e.g. with 301 redirects the links are still technically working, so we shouldn't error on them.

But is warning the best option? Can’t we check the redirected link and warn/error if it’s broken and pass otherwise? @mortenpi

@mortenpi
Copy link
Member

Can’t we check the redirected link and warn/error if it’s broken and pass otherwise?

That's an additional step, that we actually follow the 3XX redirects. We could definitely do that if someone wants to take a stab at implementing this.

@charleskawczynski
Copy link
Author

Ok. Should we reopen this issue, then?

@mortenpi
Copy link
Member

Perhaps a new one? The DOI/302 issue has been resolved (temporary redirects do not print a warning nor an error).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants