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

Link checker #2543

Merged
merged 5 commits into from
Nov 22, 2022
Merged

Link checker #2543

merged 5 commits into from
Nov 22, 2022

Conversation

alflennik
Copy link
Contributor

Adds a link checker which will run in CI. It can also be run locally with npm run link-checker.

  • In addition to anchor links, it also checks stylesheet, script and image links.
  • In addition to checking the HTTP status, it also checks that hash links are pointing to an id on the page which exists.
  • It supports the link rewriting which is done for spec links.
  • There is a configuration file (".link-checker.js") which supports excluding links on a particular page with a CSS selector (useful for some examples) and a couple other controls.

Huge thanks to @Paul-Clue at Bocoup who worked with me on this feature from beginning to end.

@alflennik alflennik requested a review from mcking65 November 16, 2022 20:09
@nschonni
Copy link
Contributor

See #2147 for a simpler solution than a custom script

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@nschonni, did you review the list of things being tested? My understanding was that #2147 does not support all the types of references we need to test.

@nschonni
Copy link
Contributor

https://github.com/JustinBeckwith/linkinator seems to have all that. You could also use the extra logic in the linksToSkip for the extra skipping you found was needed. My PR was against the current main, and I haven't tried rebasing it against this branch.

@alflennik
Copy link
Contributor Author

Apologies @nschonni I didn't realize you had a PR open which covered the same feature that Paul and I were working on. If we knew that we would have touched base earlier.

We did try Linkinator as we were authoring the PR, but we were a bit irked that it did not check hash links (see issue), which is a limitation off-the-shelf link checkers seem to have, and this was the initial reason that led us to decide to implement a custom link checker.

@nschonni
Copy link
Contributor

No problem at all! My main point with other PR was just to leverage as much externally supported code. Only suggestion might be to leverage Linkinator (or something else) if it covers 80% of the cases, to minimize the amount this project has to maintain

@mcking65
Copy link
Contributor

I suggest we satisfy our immediate need with this custom script.

Perhaps as a long term maintenance strategy, could we fork linkenator, add missing capability, offer it back in a PR, and then opefully be able to use it in the future? Would be interested in sizing that next year.

Base automatically changed from move-examples-build-fixes to move-examples November 22, 2022 19:24
@alflennik alflennik merged commit bb45ea4 into move-examples Nov 22, 2022
@alflennik alflennik deleted the link-checker branch November 22, 2022 19:26
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.

3 participants