Conversation
It seems the magic which converts `README.md` paths to `index.html` doesn't work here, because the generated path is to a non-existent `README.html`.
alexdewar
force-pushed
the
fix-broken-links
branch
2 times, most recently
from
86743c2 to
4505241
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1147 +/- ##
=======================================
Coverage 87.52% 87.52%
=======================================
Files 55 55
Lines 7626 7626
Branches 7626 7626
=======================================
Hits 6675 6675
Misses 649 649
Partials 302 302 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
alexdewar
force-pushed
the
fix-broken-links
branch
from
90899ec to
237de2b
Compare
| \\( \sum_t act_t \\). | ||
|
|
||
| [overall MUSE2 workflow]: ./README.md#framework-overview | ||
| [framework-overview]: https://energysystemsmodellinglab.github.io/MUSE2/model/index.html#framework-overview |
There was a problem hiding this comment.
Why do we have to use a URL here rather than an internal link?
There was a problem hiding this comment.
Pull request overview
This pull request fixes issues with the link checker workflow that were causing CI failures. The changes address broken internal documentation links, spurious external link failures, and add exclusions for problematic auto-generated files.
Changes:
- Fixed two broken internal links in
docs/model/investment.mdby converting them to absolute URLs pointing to the published documentation - Added
.lycheeignoreentry to skip erased-serde documentation links that incorrectly return 404s - Extended the link checker workflow to exclude the auto-generated
book/404.htmlfile
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/model/investment.md | Consolidated and converted two relative links to README section into a single absolute URL reference |
| .lycheeignore | Added pattern to ignore problematic erased-serde docs.rs links |
| .github/workflows/check-docs.yml | Added exclusion pattern for auto-generated 404.html file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| \\( \sum_t act_t \\). | ||
|
|
||
| [overall MUSE2 workflow]: ./README.md#framework-overview | ||
| [framework-overview]: https://energysystemsmodellinglab.github.io/MUSE2/model/index.html#framework-overview |
There was a problem hiding this comment.
Using an absolute URL to the published documentation creates a hard dependency on the documentation hosting location and won't work for local builds or forks.
The better approach would be to use a relative path that works with mdBook's build system. Since model/README.md becomes model/index.html when built, you could try:
./README.md#framework-overview(should work in raw markdown and mdBook)- Or configure the link checker to understand mdBook's README→index.html transformation
The current absolute URL will break if the documentation is ever moved to a different domain or viewed locally, and it creates an external dependency where an internal link would suffice.
| [framework-overview]: https://energysystemsmodellinglab.github.io/MUSE2/model/index.html#framework-overview | |
| [framework-overview]: ./README.md#framework-overview |
| uses: lycheeverse/lychee-action@v2 | ||
| with: | ||
| args: "--verbose --no-progress --accept 200,203,503 --exclude-path ^target './**/*.md' './**/*.html'" | ||
| args: "--verbose --no-progress --accept 200,203,503 --exclude-path ^target --exclude-path ^book/404.html$ './**/*.md' './**/*.html'" |
There was a problem hiding this comment.
The pattern ^book/404.html$ might not match the file path as lychee sees it. The existing pattern ^target is a prefix match (no $ anchor), while your new pattern anchors to both start and end.
Depending on how lychee resolves paths, you might need ^book/404\.html$ (with escaped dot), or ^.*book/404\.html$ to allow for any prefix. Consider testing this locally to ensure it actually excludes the intended file. If it doesn't work, you might need to use .lycheeignore instead of --exclude-path.
| args: "--verbose --no-progress --accept 200,203,503 --exclude-path ^target --exclude-path ^book/404.html$ './**/*.md' './**/*.html'" | |
| args: "--verbose --no-progress --accept 200,203,503 --exclude-path ^target --exclude-path ^.*book/404\.html$ './**/*.md' './**/*.html'" |
Description
The link checker workflow is currently not working, which is causing other open PRs to fail CI checks.
There are a few reasons:
docs/model/investment.mderased-serdecrate that are linked to from our API docs, even though it should be generated automatically. Not sure what the problem is, but I've just ignored these for now.book/404.htmlfile, which is also autogenerated, so I'm just ignoring the file nowThe link checker workflow doesn't actually run on pushes to
main. We are checking that the docs build ok, as part of the docs deployment workflow, but maybe we should also check links here too.Fixes # (issue)
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks