Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce Markdown linter and formatter #2184

Merged

Conversation

opeco17
Copy link
Contributor

@opeco17 opeco17 commented Sep 2, 2023

What this PR does / why we need it:

This PR introduces markdownlint-cli2, the linter and formatter for Markdown files.
make lint will be able to check the format of Markdown files and make lint-fix fix them.

Since some rules can't be fixed by markdownlint-cli2 automatically, I've disabled those rules temporarily.
They should be enabled one by one once it's fixed.

How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)
does not change cardinality

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2178

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 2, 2023
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 2, 2023
@opeco17 opeco17 force-pushed the feat/introduce-markdown-formatter branch from 700766b to efc5dce Compare September 2, 2023 06:07
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2023
@opeco17 opeco17 force-pushed the feat/introduce-markdown-formatter branch from efc5dce to d14d661 Compare September 2, 2023 06:11
Makefile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/networkpolicy-metrics.md Outdated Show resolved Hide resolved
@opeco17 opeco17 force-pushed the feat/introduce-markdown-formatter branch from d14d661 to 3efe474 Compare September 2, 2023 07:45
@mrueg
Copy link
Member

mrueg commented Sep 5, 2023

Thanks, this is looking good! Two things from my side:

  • I would want to understand when a list is - and when it's okay to use *. Do you know which criteria the linter has?
  • Can you add it as a github action job ci-markdown-lint?

@opeco17 opeco17 force-pushed the feat/introduce-markdown-formatter branch from 3efe474 to fadb8b3 Compare September 6, 2023 12:30
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 6, 2023
@opeco17
Copy link
Contributor Author

opeco17 commented Sep 6, 2023

Hi, @mrueg
Thank you for the comment

I would want to understand when a list is - and when it's okay to use *. Do you know which criteria the linter has?

According to this document, it's possible to use * by setting a parameter in jsonc .
I've updated that setting and formatted Markdown files.

Can you add it as a github action job ci-markdown-lint?

Linter to Markdown is called in make lint command.
So it can run in CI without updating ci.yaml

lint: shellcheck licensecheck lint-markdown-format
	golangci-lint run

@dgrisonnet
Copy link
Member

/triage accepted
/assign @mrueg @dgrisonnet

@k8s-ci-robot k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Sep 7, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Sep 7, 2023
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@mrueg
Copy link
Member

mrueg commented Sep 11, 2023

Thanks, a small couple of nits in the Makefile. Other than that looks good to me.

@opeco17
Copy link
Contributor Author

opeco17 commented Sep 11, 2023

@mrueg

Thank you for your suggestions!
I've merged all of them

@opeco17 opeco17 force-pushed the feat/introduce-markdown-formatter branch from 9093d90 to 81e825d Compare September 11, 2023 13:42
@mrueg
Copy link
Member

mrueg commented Sep 11, 2023

/lgtm

Thanks for your contribution!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 11, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrueg, opeco17

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 11, 2023
@k8s-ci-robot k8s-ci-robot merged commit a7cb7f7 into kubernetes:main Sep 11, 2023
12 checks passed
@opeco17 opeco17 deleted the feat/introduce-markdown-formatter branch September 11, 2023 21:54
@opeco17 opeco17 mentioned this pull request Sep 12, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add linter to the markdown docs
4 participants