ci(links-check): add external link checker using linkinator-action #6386
Conversation
Adds external link validation to spot dead links (status code != 200) while keeping existing internal link checker. Fixes fastify#6385
There was a problem hiding this comment.
Well I checked the sourcecode of linikator and linkinator-action and it seems trustable.
|
CI is green here while many broken links were found on #6394 |
There was a problem hiding this comment.
lgtm
CI is green here while many broken links were found on #6394
So it doesn't work @jean-michelet?
|
I guess, but I don't really know the workflow so maybe @Fdawgs can tell us more about this. |
|
I dont think the link checker workflow ran because it only triggers on markdown file changes: paths: "**/*.md"This PR initially only modified .github/workflows/links-check.yml, so the workflow never executed. to test, I've added a trivial change to README.md to trigger the workflow and verify the external link checker works correctly. |
|
We should probably fix the links here. |
|
|
||
| on: | ||
| pull_request: | ||
| paths: |
There was a problem hiding this comment.
Shouldn't we detect when the workflow file itself is updated?
".github/workflows/links-check.yml"
|
These links are flagged as "fragment not found" but are actually valid. They either use hash-based routing (SPAs) or the fragments do exist: - https://sinclairzx81.github.io/typebox/#/docs/overview/2_setup
- https://getpino.io/#/
- https://getpino.io/#/docs/api?id=opt-serializers
- https://getpino.io/#/docs/redaction
- https://github.com/fastify/fastify/issues/946#issuecomment-766319521
- https://github.com/openjs-foundation/cross-project-council/blob/HEAD/PROJECT_PROGRESSION.md#at-large-projects
- https://github.com/fastify/fastify/blob/94ea67ef2d8dce8a955d510cd9081aabd036fa85/test/hooks-async.js#L269-L275
- https://github.com/galiprandi/fastify-lm#readme
- https://github.com/fastify/fastify/issues/1426#issuecomment-817957913
- https://github.com/pinojs/pino/blob/main/docs/api.md#level-string
- https://github.com/pinojs/pino/blob/main/docs/api.md#serializers-object
- https://github.com/pinojs/pino/blob/main/docs/api.md#optionsI still expect the link checks to fail on some of these due to the action detecting a fragment in the URL, even though the links are correct. I'll review the CI again and look into adding these to linksToSkip or create a regex if needed. Question re - https://www.letzdoitapp.com/ -> This domain appears to no longer exist. What should we do here - remove the link entirely, or keep the name but remove the hyperlink? |
|
I suspect linkinator reports fragment links as broken when the corresponding id isn’t present in the HTML. |
|
we have this PR too: #6424 Not yet checked this PR to evaluate the differences tho |
There was a problem hiding this comment.
linkinator is a good lib, the maintainer is active too, I think we can proceed with this, but CI needs to be fixed
CONTRIBUTING.md
Outdated
| file. This list is also sorted alphabetically so make sure to add your name | ||
| in the proper order. Use your GitHub profile icon for the `picture:` field. | ||
| 5. Read the [pinned announcements](https://github.com/orgs/fastify/discussions/categories/announcements) | ||
| 5. Read the [pinned announcements](https://github.com/fastify/fastify/discussions/categories/announcements) |
There was a problem hiding this comment.
this must not change, see the linksToSkip suggestion
There was a problem hiding this comment.
updated here - 2359f2f
we have the following in link-checks.yml so i dont think we need to add the complete url to this property
linksToSkip: "https://github.com/orgs/fastify/.*"
EXPENSE_POLICY.md
Outdated
| - Hosting | ||
|
|
||
| **Before making any purchases**, initiate a [new discussion](https://github.com/orgs/fastify/discussions) | ||
| **Before making any purchases**, initiate a [new discussion](https://github.com/fastify/fastify/discussions) |
There was a problem hiding this comment.
this must not change, see the linksToSkip suggestion
There was a problem hiding this comment.
updated here - 2359f2f
we have the following in link-checks.yml so i dont think we need to add the complete url to this property
linksToSkip: "https://github.com/orgs/fastify/.*"| - [Platformatic](https://platformatic.dev) | ||
|
|
||
| Past Sponsors: | ||
| - [LetzDoIt](https://www.letzdoitapp.com/) |
There was a problem hiding this comment.
This link is dead, why it has not been catched?
I think we need to update the paths
There was a problem hiding this comment.
Updated here - 9750c33 the paths trigger from '.md' to '**/.md' so the workflow runs when any markdown file in the repository is modified (including those in subdirectories like test/bundler/README.md).
I checked the various PR and I think this one is good to go with some suggestions 👍🏼 |
Co-authored-by: Manuel Spigolon <behemoth89@gmail.com> Signed-off-by: Umar Gora <umarg1997@gmail.com>
Co-authored-by: Manuel Spigolon <behemoth89@gmail.com> Signed-off-by: Umar Gora <umarg1997@gmail.com>
Co-authored-by: Manuel Spigolon <behemoth89@gmail.com> Signed-off-by: Umar Gora <umarg1997@gmail.com>
Adds external link validation to spot dead links (status code != 200)
while keeping existing internal link checker.
Fixes #6385
Checklist
npm run test && npm run benchmark --if-presentand the Code of conduct