Markdown link check in CI #2543
Conversation
| # markdown-link-check doesn't support multiple files on the commandline, so this hacks around that. | ||
| # Note that you must install the package separately via npm. For example: | ||
| # brew install npm; npm install -g markdown-link-check | ||
| entry: bash -xc 'echo "$@" | xargs -n1 -t markdown-link-check -c markdown-link-check.config.json' -- |
There was a problem hiding this comment.
Cribbed from pre-commit/pre-commit#394 (comment)
| language: system | ||
| types: [markdown] | ||
| # Don't check localized files since their target might not be localized. | ||
| exclude: ".*localized.*" |
There was a problem hiding this comment.
This might be controversial - if a localized file tries to link to something that hasn't been translated, what should we do? We could link to the english version, but that might be bad for the user. Currently we allow the dead links but we can change them if we really want.
There was a problem hiding this comment.
Right now for the localized file, if some doc hasn't been translated, we link them to the english version and try to avoid dead links.
There was a problem hiding this comment.
OK, let's get this merged first, then fix that in a subsequent PR
|
|
||
| markdown_link_check: | ||
| docker: | ||
| - image: circleci/node:12.6.0 |
There was a problem hiding this comment.
Why are we running this check in a separate docker image, and in a separate job (1. build and test 2. markdown_link_check) instead of in the same docker image in the same job?
There was a problem hiding this comment.
I mentioned in the PR description
and installing npm on a python container in CircleCI didn't seem to work.
I'll give it another try though (unfortunately it's a lot of trial and error, as you can tell by the number of commits on this PR)
There was a problem hiding this comment.
Ah sorry I missed that part. In that case I think two jobs is fine. (No need to spend your time getting it to work within one job. )
There was a problem hiding this comment.
Tried again in #2547 but installation doesn't worth without sudo, and then node cant' find the file when running.
At this point I'd rather just rewrite the link check in python so we have more control over it, and run it directly from pre-commit. But I think the current approach will work fine for now.
* check using xargs * fix broken BC link * install npm, run precommit before unit tests * try to install npm * try a node image build * add workflow * don't use precommit on node run * sudo make me a sandwich * pass config arg * revert CI order change * retry precommit * sudo apt-get * sudo npm * make sure fails on bad link * cleanup and refix link
Enable markdown link checking in CI.
This should have been easy, but
This means we can't use
language: nodein pre-commit, and installingnpmon a python container in CircleCI didn't seem to work. So what we're left with is: