Community: Markdown linter for all of K8s org

[jonasrosland]

This is a suggestion based on some of the great work was done last week by @akutz with regards to making sure Markdown docs are properly linted. This could help the project's more Markdown heavy locations become more standardized, and also help drive and apply parts of the style guide.

The first PR that went into effect is here: https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/pull/425
And then it was added as a pre-submit here: https://github.com/kubernetes/test-infra/pull/13363

Perhaps this could see wider use in the K8s org?

/assign @cblecker
/sig contribex
/kind documentation

[akutz]

Thank you @jonasrosland. For those reading this, here is some additional information regarding the linting process:

• .markdownlintrc - A file at the root of a repository that influences the linter
• Linter rules - The rules that may be enabled/disabled with the rc file

• It's possible to run the linter manually on existing k/k projects. For example, here's a gist where the linter is executed against the client-go repository with:

  cd $(mktemp -d -p /tmp) && \
  git clone https://www.github.com/kubernetes/client-go . && \
  docker run --rm -v "$(pwd)":/build:ro gcr.io/cluster-api-provider-vsphere/extra/mdlint
  

• The image gcr.io/cluster-api-provider-vsphere/extra/mdlint is built from this Dockerfile and is based on Google's distroless, nodejs image.

• Please note that while the job spec is pretty simple, it's necessary to explicitly specify the entry point and arguments that are the default values inside the image. This is due to the way Prow works.

  spec:
    containers:
    - image: gcr.io/cluster-api-provider-vsphere/extra/mdlint:0.17.0
      command:
      - /nodejs/bin/node
      args:
      - /md/lint
      - -i
      - vendor
      - .
  

[cblecker]

/help

[k8s-ci-robot]

@cblecker:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tao12345666333]

Thank you. I have tried adding markdown lint to kubernetes-sigs/kind https://github.com/kubernetes-sigs/kind/pull/1122

[BenTheElder]

Personally I appreciate the work on this but I'm concerned about potentially adding another barrier to successfully PRing changes that blocks on things that don't really affect the content rendering or readability.

Also cc @zacharysarah @jimangel to tag in some
/sig docs

Apologies for the long winded comment, but I'd like to be specific and clear on this one:

---

Linters are kinda awful for new contributors because they have to figure out how to tell what is failing and why from our CI (which they've probably never seen before, or this tool)... which is actually not so easy with the log highlighting etc... and then how to run it locally....

I'm willing to accept that barrier when we're doing static analysis to help catch breaking bugs that we'd ideally catch in human review anyhow (but frequently don't because humans get tired and bugs can be subtle) like:

for i := range foo {
    // whoops forgot to capture i, linter will catch this, hopefully a reviewer would have too!
    go func() {
        fmt.Println(i)
    }
}

But not as much when we're doing:

* a 
* b 

versus:

- a
- b

Which IMO are both equally readable and produce the same rendered content, I.E. neither form contains "bugs" (Not a complete markdown expert, I could be wrong ...)

Automated formatters are not so bad but still a hurdle to jump through, especially for e.g. contributors on Windows when we use bash all over ...

kind frequently sees totally new contributors (sometimes their first use of github!) sending drive-by PRs to fixup documentation that tripped them up, I'd hate to see their PRs blocked on stray end of-line spaces that don't really affect rendering or readability :(

Looking at https://github.com/kubernetes-sigs/kind/pull/1122, almost none of the changes affect the content at all and the original content is equally readable imho.

Having good style is certainly nice, but for kind at least I'd much prefer helpful content that is readable enough and a smooth contributing experience over fighting with robots that bikeshed every last character, if that makes sense ...

---

EDIT: A small addendum ...

> In general, reviewers should favor approving a CL once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn’t perfect.

I think we should really try more to do this. I'm trying to myself.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[cblecker]

/remove-lifecycle stale

[mrbobbytables]

I'm inclined to agree with @BenTheElder. I'm a stickler for markdown formatting and conforming with the style guide where it has been used, but I'd rather not force it on folk ESPECIALLY org wide when it'd likely just be a burden for folk when attempting to improve documentation. We'd also likely have a slew of things we'd have to add to the whitelist like the various retro notes that are converted to markdown from gdoc.

We could potentially enable it selectively for things like our contributor docs, but that would very much be targeted and limited to things that generally break markdown formatting (e.g. MD032 - Lists should be surrounded by a blank line).

[qq843700497]

I want to be a new contributor, I agree with @BenTheElder

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[mrbobbytables]

Following up on this item, we did come to the conclusion that we should not move forward with an org wide linter. 👍
I'm going to go ahead and close it out, but If you have any questions or concerns please feel free to reopen :)

/close

[k8s-ci-robot]

@mrbobbytables: Closing this issue.

In response to this:

Following up on this item, we did come to the conclusion that we should not move forward with an org wide linter. 👍
I'm going to go ahead and close it out, but If you have any questions or concerns please feel free to reopen :)

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.