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

🌱 Change link checker to lychee #517

Merged

Conversation

peppi-lotta
Copy link
Member

@peppi-lotta peppi-lotta commented Nov 11, 2024

Current Broken-link-checker is not actually checking in the https://metal3.io/ webpage. It is causing a lot of flakes like this:
#448
#485

In current link checker no URL is specified so it uses the default https://github.com/${{ github.repository }}{}. This means we are checking links in the site https://github.com/metal3-io/metal3-io.github.io instead of the intended https://metal3.io/ . To check all pages in a site we should serve a sitemap.xml as input to be checked.

I also think workflow should move to use lychee
https://lychee.cli.rs/introduction/#check-all-links-on-a-website
https://github.com/lycheeverse/lychee-action

Lychee is better documented, has active maintainers and widely adopted. Here is a workflow for that.

I've tested this code in my own fork. It creates quite nice reports and issues:
https://github.com/peppi-lotta/metal3-io.github.io/actions/runs/11834513843/job/32975333756
peppi-lotta#27

Copy link

netlify bot commented Nov 11, 2024

Deploy Preview for metal3io-website ready!

Name Link
🔨 Latest commit f2bd337
🔍 Latest deploy log https://app.netlify.com/sites/metal3io-website/deploys/6737123dbe87b20008faa20f
😎 Deploy Preview https://deploy-preview-517--metal3io-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@metal3-io-bot metal3-io-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 11, 2024
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/change-link-checker branch 2 times, most recently from f7fc30f to 3ece798 Compare November 11, 2024 15:05
@tuminoid
Copy link
Member

Report is nice, but it is full of false positives. If you look at it, it fails to local files, template variables etc which are not right. It seems to be parsing source rather than the live website?

/hold

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 11, 2024
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/change-link-checker branch 6 times, most recently from 822e8f4 to 1ba7ec1 Compare November 14, 2024 09:36
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
.github/workflows/broken-link-check.yml Outdated Show resolved Hide resolved
Signed-off-by: Peppi-Lotta <[email protected]>
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/change-link-checker branch from 1ba7ec1 to f2bd337 Compare November 15, 2024 09:19
id: lycheemetal3io
uses: lycheeverse/lychee-action@f81112d0d2814ded911bd23e3beaa9dda9093915 # v2.1.0
with:
args: --user-agent "Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0" ${{ steps.extract.outputs.urls }}
Copy link
Member

Choose a reason for hiding this comment

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

I love how the user-agent is some 15 year old build of Firefox ;)

Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

IMO this is now worth a go.

/lgtm
/unhold

@metal3-io-bot metal3-io-bot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Nov 15, 2024
@tuminoid
Copy link
Member

/cc @lentzi90

Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

Looks like a solid improvement!
/approve
One question below
/hold

http://localhost
https://0.0.0.0
http://0.0.0.0
https://github.com/issues
Copy link
Member

Choose a reason for hiding this comment

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

Why ignore GH issues?

Copy link
Member

@tuminoid tuminoid Nov 18, 2024

Choose a reason for hiding this comment

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

Scanner gets detected by github as bot, and gets 404. Bots should be using GH API for issue search.

Copy link
Member

Choose a reason for hiding this comment

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

The actual link failing is filtered query to metal3 issues, IIRC good first issues. Its not the base issue url that we technically ignore.

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2024
@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lentzi90

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 18, 2024
@tuminoid
Copy link
Member

/retitle 🌱 Change link checker to lychee

I'll unhold, it cannot be any worse than old one ;-)
/unhold

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2024
@metal3-io-bot metal3-io-bot changed the title 🌱 Change link checker 🌱 Change link checker to lychee Nov 18, 2024
@metal3-io-bot metal3-io-bot merged commit 36a08c1 into metal3-io:source Nov 18, 2024
7 checks passed
@metal3-io-bot metal3-io-bot deleted the peppi-lotta/change-link-checker branch November 18, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants