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

Replace markdown-link-checker with Linkspector #85

Merged
merged 4 commits into from
Jan 28, 2025
Merged

Conversation

alexdewar
Copy link
Contributor

@alexdewar alexdewar commented Jan 20, 2025

I was looking at adding a link checker to another project and thought it would be worth checking out linkspector as an alternative to the existing markdown-link-checker action we're using now. The project's by the same author and on the markdown-link-checker repo it suggests using linkspector instead.

I thought the blog would be a good place to try it out because it contains a bunch of links.

Linkspector a couple of advantages over markdown-link-checker:

  1. It uses a headless version of Chrome to resolve the links, so the rate of false positives (i.e. good links failing) should be much lower
  2. It comments on your PR on the line where the failing link is, which is handy

I'm currently using the ubuntu-22.04 runner cf. ubuntu-latest as there's an open bug affecting the latter. Looks like there's a fix in the works though.

@alexdewar alexdewar marked this pull request as ready for review January 20, 2025 12:15
Copy link
Contributor

@AdrianDAlessandro AdrianDAlessandro left a comment

Choose a reason for hiding this comment

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

I'm not convinced it's working. I would still expect it to fail on the websites that have a CAPTCHA test and it's not failing.

Can you put in a link that we know for sure is broken?

@alexdewar alexdewar marked this pull request as draft January 27, 2025 12:10
README.md Outdated
@@ -11,9 +11,10 @@ This repository contains blogs by the [central RSE team](https://www.imperial.ac
- Install the dependencies `poetry install`
- Install the pre-commit hooks `poetry run pre-commit install`

[here is a madeup link](https://github.com/ImperialCollegeLondon/madeup_link)

Choose a reason for hiding this comment

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

🚫 [linkspector] reported by reviewdog 🐶
Cannot reach https://github.com/ImperialCollegeLondon/madeup_link. Status: 404

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AdrianDAlessandro I added a broken link just to double-check it's working ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

Wonderful! Seems good to me! I guess you can revert that commit now

@alexdewar alexdewar marked this pull request as ready for review January 27, 2025 13:44
use-verbose-mode: "yes"
github_token: ${{ secrets.github_token }}
reporter: github-pr-review
fail_on_error: true
Copy link
Contributor

Choose a reason for hiding this comment

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

I wondered whether fail_on_error meant that as soon as it found a broken link, it stopped reporting (thus not finding further broken links), so looked it up and apparently this option is deprecated (link)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot!

@dc2917 dc2917 self-requested a review January 28, 2025 11:31
@alexdewar alexdewar merged commit 64ccca7 into main Jan 28, 2025
4 checks passed
@alexdewar alexdewar deleted the new-link-checker branch January 28, 2025 12:50
@AdrianDAlessandro
Copy link
Contributor

Are you going to implement this elsewhere / in the python template?

@alexdewar
Copy link
Contributor Author

I was thinking of changing my projects to use it. That's a good shout about the Python template -- I guess we're going to be using that for the foreseeable future.

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