-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fix regressions from #2451 #2453
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
Conversation
src/utilities/process-readme.js
Outdated
@@ -1,5 +1,11 @@ | |||
const url = require('url'); | |||
|
|||
const beginsWithDocsDomainRegex = /^https?:\/\/webpack\.js\.org/; | |||
|
|||
const redirects = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhmm, I was thinking of using webpack redirects instead of this string replacement
https://github.com/webpack/webpack.js.org/blob/rebuild/webpack.prod.js#L60
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those would not be evaluated when doing file based link checking I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea is to put setup the redirect, that plugin creates meta tags with the redirect ()https://github.com/bebraw/redirect-webpack-plugin/blob/master/index.js#L15), so when hyperlink hit that link it would be redirected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might work. I'm pretty sure hyperlink models meta redirects. But it might show the redirect as an error and indicate that you should update the source url to the redirect target. We can probably mark that check as --todo
or --skip
though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would work, also, it isn't possible to ask a mantainer to update https://github.com/webpack-contrib/json-loader?
Archide repositories are git blocked or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All PR's are automatically rejected by GIthub when a repo is archived. I would certainly prefer just to update that README. I obviously already tried and got blocked
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No PRs, just ask the maintainer to push a commit updating that README.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Project README updated. I'll remove the rewriting code. Though some of the refactorings will stay, since I discovered that fragment links had the original source prepended, so some of it actually fixes a bug
I'm merging. Can't have the rebuild branch keep failing |
Marked known false negatives from hyperlink as TODO and added a url rewrite mechanism for old urls while fetching README's
This should put the
rebuild
branch in the green again