etcd
etcd copied to clipboard
etcd-io
Find a replacement for marker (markdown linter)
What would you like to be added?
Right now we're using marker as the markdown linter. However, I see two issues:
- Its functionality is limited, from its
READMEThis is a tool for finding issues in CommonMark documentation. Right now, it only identifies broken links (malformed URLs, non-existent paths, etc.).
- It's written in Rust, and while dropping our GitHub action for static checking (#18058). We wont run this linter in the Prow infrastructure as we don't have Rust/Cargo available (refer to https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/etcd-io_etcd/18058/pull-etcd-verify/1793352130300481536#1:build-log.txt%3A659-662).
While trying to fix 2, I opened a PR crawford/marker#32 to be able to use a published binary, rather than building the project itself.
However, it would be better to use a Go-written Markdown linter as alternative.
Why is this needed?
To keep linting markdown files.
@ivanvc Can we use markdownlint which is officially on Go.pkg.dev : https://pkg.go.dev/github.com/svendowideit/markdownlint https://github.com/SvenDowideit/markdownlint/blob/v0.9.4/main.go
@ivanvc Can we use markdownlint which is officially on Go.pkg.dev : https://pkg.go.dev/github.com/svendowideit/markdownlint https://github.com/SvenDowideit/markdownlint/blob/v0.9.4/main.go
It looks like an abandoned project. It was originally done by Docker, but it's now a public archive, and its last commit is from 2017. I think, ideally we should replace it with other linter that has more wide adoption.
Their README mentions that it was in use for https://docs.docker.com. However, Docker docs is now using NodeJS' markdownlint:
- https://github.com/docker/docs/blob/b952723881c6e6cd7dcc556ab4c1d5a1818ec08d/package.json#L33
- https://github.com/docker/docs/blob/b952723881c6e6cd7dcc556ab4c1d5a1818ec08d/Dockerfile#L35
Surveying GitHub doesn't seem to be promising. There are some projects, but they seem unmaintained, or abandoned: https://github.com/search?q=markdown+linter+language%3AGo&type=repositories
It's written in Rust
IMO, it's not related to RUST. It needs a release to publish executable binary. For Go ecosystem, go install can help us. However, it doesn't mean we should replace it. Just my two cents.
As far as I know, the marker is good one for the linter. I was thinking about how to add makefile to install it without cargo, mentioned by https://github.com/etcd-io/etcd/pull/16708#issuecomment-1761226782.
Is it possible to be considered as an option? upload binary to storage which support anonymous download, like
https://github.com/etcd-io/etcd/blob/bb701b9265f31d61db5906325e0a7e2abf7d3627/scripts/install-marker.sh#L12-L15
Thanks, Wei
Hey @fuweid, I agree. After researching a native-Go alternative and yielding no results, I'm leaning towards having a linter that we can use as a binary. I opened PR https://github.com/crawford/marker/pull/32 to have marker distributed as a binary. Therefore, we won't need to have cargo installed locally, nor do we need to host the compiled binary (https://github.com/etcd-io/etcd/pull/16708#issuecomment-1761226782).
However, my original point is that the use-case for marker is limited; as it states on its README, it only checks broken links. It doesn't do any other Markdown linting:
This is a tool for finding issues in CommonMark documentation. Right now, it only identifies broken links (malformed URLs, non-existent paths, etc.).
@ivanvc Nice!
However, my original point is that the use-case for marker is limited; as it states on its README, it only checks broken links. It doesn't do any other Markdown linting:
If we do have new linter requirement, maybe we can file pull request to marker. The detector for broken linters is common use case because it's easy to break it by refactor of docs.
With #18318 merged, I was looking into cleaning up scripts/install-marker.sh. But it looks like it's only used by tests/Dockerfile. Is this a Dockerfile we want to maintain? The Ubuntu version used is no longer maintained (21.10). Building the image fails because the APT repositories don't work anymore.
@jmhbnz, thoughts? (or @ahrtr, as I see you were the last to commit to that Dockerfile).
But it looks like it's only used by
tests/Dockerfile. Is this aDockerfilewe want to maintain?
I am not aware of any usage on the tests/Dockerfile. Probably we can just remove it.
Are we okay with keeping marker as the markdown linter, considering it only checks for broken links? Do we want or need to do more linting, considering that running our Markdown files against NodeJS' markdownlint-cli with the default configuration raises too many warnings?
Discussed during sig-etcd triage meeting. We recently put work into updating etcd-io/website to put a proper markdown linting workflow in place.
I am supportive of adding a new prow presubmit for etcd-io/etcd to do the same.
Once the new workflow is in place I would suggest we use a good first issue to get help from the community to address the backlog of markdown files that need updates.
Edit: For a detailed next step, we need to replicate the script and Makefile recipe for markdown linting from etcd-io/website to etcd-io/etcd Once that is done we could create the new prow job.
The first step is to set the foundation to start the markdown linting. As a reference, we're already doing it on the website (as noted in https://github.com/etcd-io/etcd/issues/18059#issuecomment-2436062946).
After we have the Makefile target, we'll need to do a new prow job, taking the website's as reference: https://github.com/kubernetes/test-infra/blob/7c58ef243f8845838f1b545c140a8ed615516441/config/jobs/etcd/etcd-website-presubmits.yaml
@Zanda256, @joshjms, we usually run FIFO. Given that you both want to contribute, we can split the work on doing the Makefile assigned to @Zanda256. And after the Makefile target is in place, @joshjms can work on the Prow job. Does that sound good?
Reopening, as we still need to merge kubernetes/test-infra#34369. And test the prow job.
It is unable to find the config file due to ETCD_ROOT_DIR being nil (instead finding from the root directory /tools/.markdownlint.jsonc).
In test_lib.sh, ETCD_ROOT_DIR is set as follows:
function set_root_dir {
ETCD_ROOT_DIR=$(go list -f '{{.Dir}}' "${ROOT_MODULE}/v3")
}
set_root_dir
Since in our markdown diff lint job, we do not expect to have go, we need a way to set ETCD_ROOT_DIR to the root directory of the code base.
Now that we have fully enabled markdownlint-cli2 in the repository, and given that it doesn't check for broken links (and marker does), I believe we could close this issue, as we're linting the markdown files and also checking for broken links.
I'll leave this issue open briefly in case someone objects. Else, I'll go ahead and close it.
Thanks everyone that put effort in this. @joshjms , @ivanvc , we meet again on the next one.