rook / rook

Storage Orchestration for Kubernetes
https://rook.io
Apache License 2.0
11.98k stars 2.64k forks source link

ci: use markdownlint to enforce mkdocs compatibility #14114

Closed BlaineEXE closed 2 weeks ago

BlaineEXE commented 3 weeks ago

mkdocs uses a markdown renderer that is hardcoded to 4 spaces per tab for detecting indentation levels, including ordered- and unordered-lists. Since we cannot easily change the renderer, begin using a markdown linter in CI that will fail if official docs do not adhere to the spacing rules.

As a starting point, the markdownlint config does not begin with the default set of checks, which might overwhelm attempts to fix them. Instead, focus on list-tab-spacing rules and a few other highly useful checks.

markdownlint also has some gaps in its abilities that allow common Rook doc issues to pass acceptance. However, it allows creating custom linting plugins. Create 2 such linting plugins to check 2 things:

For the strange lists, this is allowed and renders correctly, but it looks strange:

- first bullet
- second bullet
    still second bullet
- third bullet

    has a paragraph
    of text inside

- last bullet

Checklist:

BlaineEXE commented 3 weeks ago

@travisn @parth-gr @galexrt related to this PR (and others) https://github.com/rook/rook/issues/13968#issuecomment-2070061974

mkdocs's yaml parser is adamant that they will not support 2-spaces-per-tab, and they require 4 spaces. Mkdocs devs are adamant that they won't support modifying the parser or this particular behavior. This means that our current docs will render fine in github/vscode, but not on the website.

The best way I could find to fix our docs without switching to something other than mkdocs is to do 2 things:

As of now, I haven't fixed any lint issues. I wanted to discuss and decide how to proceed from here before doing that.

mergify[bot] commented 3 weeks ago

This pull request has merge conflicts that must be resolved before it can be merged. @BlaineEXE please rebase it. https://rook.io/docs/rook/latest/Contributing/development-flow/#updating-your-fork

BlaineEXE commented 3 weeks ago

This is a specific example of where the docs (before this change) had correct-for-github yaml that was not rendering properly using mkdocs.

1. Paste the above output from `create-external-cluster-resources.py` into your current shell to allow importing the source data.

2. The import script in the next step uses the current kubeconfig context by
   default. If you want to specify the kubernetes cluster to use without
   changing the current context, you can specify the cluster name by setting
   the KUBECONTEXT environment variable.

   \`\`\`console
   export KUBECONTEXT=<cluster-name>
   \`\`\`

3. Run the [import](https://github.com/rook/rook/blob/master/deploy/examples/import-external-cluster.sh) script.

   !!! note
       If your Rook cluster nodes are running a kernel earlier than or equivalent to 5.4, remove
       `fast-diff,object-map,deep-flatten,exclusive-lock` from the `imageFeatures` line.

    \`\`\`console
    . import-external-cluster.sh
    \`\`\`

The first code block is 3 spaces from the left but is being rendered as though it were fully on the left. This also resets the numbering. This is a case where we think one thing will happen but another thing happens instead.

The callout/admonition is 3 spaces from the left, and this results in the callout not being rendered properly at all.

Further, even though the code block that follows it is 4 spaces from the left, because the callout was only indented 3 spaces, the callout is rendered as though it were also fully on the left. If there were more numbered items to follow, they would also reset to 1 because of this error.

In all cases, forcing 4-space tabs in our docs ensures that what we write in .md translates the way we expect when mkdocs outputs it to HTML. There could be other corner cases, but I believe this will fix 99% of our current docs bugs that are caused by mkdocs' reliance on 4-space tabs.

image

mergify[bot] commented 3 weeks ago

This pull request has merge conflicts that must be resolved before it can be merged. @BlaineEXE please rebase it. https://rook.io/docs/rook/latest/Contributing/development-flow/#updating-your-fork

BlaineEXE commented 2 weeks ago

Latest updates (from Friday)

For consideration:

The main reason why I also included 'Prettier' was because markdownlint didn't have a good way of checking that all lines are indented to a 4-space boundary (for mkdocs)

Now that I know how to write custom markdownlint modules, it would be fairly simple to write one to ensure this 4-space boundary

This would allow us to get rid of 'Prettier' if we want, with some pros/cons: