ci: check internal links in mdbook after build #3015
Conversation
not-my-profile
force-pushed
the
ci-hyperlink
branch
7 times, most recently
from
43c774e
to
886ceef
Compare
I want to introduce another link checking job that works on the HTML output, so I am renaming our existing link check job to disambiguate it.
not-my-profile
force-pushed
the
ci-hyperlink
branch
from
886ceef
to
e695fd2
Compare
| @@ -92,7 +92,7 @@ jobs: | |||
| # Check out `test-all.yaml` for more ideas on organizing these. | |||
| if: always() | |||
| needs: | |||
There was a problem hiding this comment.
I think it would be nice to include the new check-links-book job under check-ok-to-merge.needs ... however I guess then we could no longer use on.pull_request.paths in the new workflow file? And would probably have to do something like what's described in this blog post?
There was a problem hiding this comment.
Yes, unfortunately I'm not sure it's easy to do this...
There was a problem hiding this comment.
Just saw what you did here — nice! I had seen this action before, but hadn't tried it out yet. Looks good; we could think about extending to more places if it turns out well.
| - name: Build the mdbook | ||
| run: mdbook build web/book/ | ||
|
|
||
| - name: Check links |
There was a problem hiding this comment.
One option would be to add this to the existing workflow, or to call the existing workflow as part of this. But no strong view.
There was a problem hiding this comment.
Though possibly moving the job into build-web.yaml would be a reasonable consolidation — it would run slightly too often, but not by much.
There was a problem hiding this comment.
I am planning on linking relevant pages of the book from doc comments in prql_ast in the future ... at which point I want to expand this CI job to check these links as well ... so I think it would be best to keep these separate since they have different path requirements and do largely different things.
not-my-profile
force-pushed
the
ci-hyperlink
branch
from
4393db0
to
2331008
Compare
| - name: Cache | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| prefix-key: 0.8.1 |
There was a problem hiding this comment.
FYI, prefix-key should be updated by the Cargo.toml.
prql/crates/prql_compiler/Cargo.toml
Lines 78 to 85 in 0224b6e
There was a problem hiding this comment.
Ah, missed that, thanks @eitsupi
We already run
markdown-link-checkin our CI, however by virtue of it running against.mdfiles it cannot detect links that are broken due to mdbook-specifics (such as forgetting to list the .md file in SUMMARY.md or the README.md bug). I already fixed 5 such broken links in #3017 and #2990 ... we should really detect these broken links in the CI.This PR therefore adds a new
link-check-bookjob to our CI that builds the mdbook and then checks the generated HTML for broken links with the hyperlink link checker.