Don't linkcheck external web links in PR CI #2296
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jieyouxu
commented
Mar 19, 2025
book.toml
Outdated
Comment on lines
31
to
54
| exclude = [ | ||
| "crates\\.io", | ||
| "gcc\\.godbolt\\.org", | ||
| "youtube\\.com", | ||
| "youtu\\.be", | ||
| "dl\\.acm\\.org", | ||
| "cs\\.bgu\\.ac\\.il", | ||
| "www\\.amazon\\.com", | ||
| "www\\.rustaceans\\.org", | ||
| "play\\.rust-lang\\.org", | ||
| "tomlee\\.co", | ||
| "marketplace\\.visualstudio\\.com", | ||
| "objects\\.githubusercontent\\.com", | ||
| # The bug listing URL works only if an user is logged in, otherwise one gets 404. | ||
| "github\\.com/issues\\?q=.*", | ||
| # Similarly 500 is sometimes returned here. | ||
| "github\\.com/rust-lang/rust/pulls\\?q=.*", | ||
| # 401 is returned here for unknown reason | ||
| "github\\.com/wesleywiser/rustc-bootstrap-wpa-analysis", | ||
| # Handle: connection closed before message completed | ||
| "microsoft\\.com/en-us/research/publication/", | ||
| # 500 is returned for HEAD request | ||
| "code\\.visualstudio\\.com/docs/editor/tasks", | ||
| ] |
There was a problem hiding this comment.
This long list of excludes to me suggests that it's not good in terms of signal-to-noise ratio, because we're already skipping the (arguably most import) github links (issues, PRs, commits, etc.).
jieyouxu
added
S-waiting-on-review
|
We can set (But I'm also fine with disabling it altogether, tbh. Up to you). |
jieyouxu
force-pushed
the
no-external-linkcheck
branch
from
98d7c48 to
5d91437
Compare
jieyouxu
commented
Mar 19, 2025
jieyouxu
changed the title
|
Actually, I'll skip for PR CI only for now. |
jieyouxu
force-pushed
the
no-external-linkcheck
branch
from
5d91437 to
c9210de
Compare
jieyouxu
changed the title
jieyouxu
force-pushed
the
no-external-linkcheck
branch
from
c9210de to
637ad77
Compare
|
Can you try to commit a change to some random MD that contains a non-existent third-party URL? It has to start with http/https. |
|
💀 |
|
Oh is it because this shell script is a wrapper around the linkchecker not mdbook... IDK |
jieyouxu
force-pushed
the
no-external-linkcheck
branch
from
8301c7e to
aa73732
Compare
jieyouxu
force-pushed
the
no-external-linkcheck
branch
from
aa73732 to
531e5f9
Compare
|
(Looks like that works) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This can block PR CI yet remain unactionable for the contributor, since this is subject to network conditions and availability of the servers hosting the external web links.
I don't see a way for PR CI to override the linkcheck config to skip external web links yet let the cron job CI to perform external web link checks, so this PR does the sledgehammer approach of disabling external web link checks for both.EDIT: apparentlyMDBOOK_OUTPUT__LINKCHECK__FOLLOW_WEB_LINKSis a thing.