Skip to content

feat: add markdownlint for md documents#1417

Merged
kmesh-bot merged 8 commits intokmesh-net:mainfrom
brushax:md-lint
May 23, 2025
Merged

feat: add markdownlint for md documents#1417
kmesh-bot merged 8 commits intokmesh-net:mainfrom
brushax:md-lint

Conversation

@brushax
Copy link
Copy Markdown
Contributor

@brushax brushax commented May 22, 2025

What type of PR is this?

/kind enhancement

What this PR does / why we need it:

use markdownlint(docker version) to lint markdown documents.

Currently there are many problems reported by markdownlint, so overwriting is disabled.

Which issue(s) this PR fixes:
Fixes #1331

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Copilot AI review requested due to automatic review settings May 22, 2025 15:50
@kmesh-bot kmesh-bot added the kind/enhancement New feature or request label May 22, 2025
@kmesh-bot
Copy link
Copy Markdown
Collaborator

Welcome @Flying-Tom! It looks like this is your first PR to kmesh-net/kmesh 🎉

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds markdownlint step to hack/format.sh for linting Markdown files, and tightens quoting on install commands and format tool invocations.

  • Quote install tool arguments for safety
  • Correct formatting commands to operate on the intended directory
  • Introduce a Docker-based Markdown linting step
Comments suppressed due to low confidence (2)

hack/format.sh:29

  • The gofmt command now only formats files in hack/ rather than the repository root. If the intent is to format the entire project, revert to gofmt -w -s .. or adjust the working directory before running this command.
gofmt -w -s ./

hack/format.sh:33

  • Similar to gofmt, shfmt now only formats the hack/ directory. To ensure shell scripts across the project are formatted, consider targeting the repo root (e.g., ../) or changing into the root directory first.
shfmt -w -s -ln=bash ./

@brushax brushax force-pushed the md-lint branch 2 times, most recently from a94e5c9 to f5c6488 Compare May 22, 2025 15:54
@yp969803
Copy link
Copy Markdown
Contributor

i think it is only checking not formatting the files

@brushax
Copy link
Copy Markdown
Contributor Author

brushax commented May 22, 2025

i think it is only checking not formatting the files

Yes, becaused there are so many problems so there are no fix currently. After more discussion, I will proceed on it.

@yp969803
Copy link
Copy Markdown
Contributor

ok

@kmesh-bot kmesh-bot added size/XXL and removed size/S labels May 22, 2025
Signed-off-by: Tom <yusencao@outlook.com>
@brushax brushax force-pushed the md-lint branch 2 times, most recently from 9d9639b to cc3bb34 Compare May 22, 2025 18:20
@brushax
Copy link
Copy Markdown
Contributor Author

brushax commented May 22, 2025

Doc generated by make gen have lint problems and need to be ignore.

@codecov
Copy link
Copy Markdown

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.33%. Comparing base (b7848f1) to head (7bd0880).
Report is 12 commits behind head on main.

see 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98facf9...7bd0880. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yp969803
Copy link
Copy Markdown
Contributor

.PHONY: gen
gen: tidy
gen-proto
gen-bpf2go
gen-kmeshctl-doc
format

generation happens first then format, so it should also format generated files @Flying-Tom

@brushax
Copy link
Copy Markdown
Contributor Author

brushax commented May 23, 2025

.PHONY: gen gen: tidy gen-proto gen-bpf2go gen-kmeshctl-doc format

generation happens first then format, so it should also format generated files @Flying-Tom

I thought that is the problem, the documents generated violates some lint rules, and disabling them in .markdownlint.yml will make the global check to be too loose.

From another perspective, I don't think markdown format(actually lint) is neccessary in CI, unless insisting on the check of markdown documents in CI. The CI does't upload any files or update the repo, it only perform the lint check, not the reformat. I prefer to use make format locally.

@yp969803
Copy link
Copy Markdown
Contributor

yp969803 commented May 23, 2025

we use make format locally, at CI we run make format to catch any change, and show to the users that is to be done, it is better if we run format mardown at CI also

@brushax
Copy link
Copy Markdown
Contributor Author

brushax commented May 23, 2025

I think you are right, format markdown should only run locally, and at CI we should check the lint issues

I think markdown format(lint) may be needed actually, but should not be int the big CI. There can be another CI, just like lint the perform style check.

brushax added 6 commits May 23, 2025 11:44
Signed-off-by: Tom <yusencao@outlook.com>
Signed-off-by: Tom <yusencao@outlook.com>
Signed-off-by: Tom <yusencao@outlook.com>
Signed-off-by: Tom <yusencao@outlook.com>
Signed-off-by: Tom <yusencao@outlook.com>
Signed-off-by: Tom <yusencao@outlook.com>
Signed-off-by: Tom <yusencao@outlook.com>
@hzxuzhonghu
Copy link
Copy Markdown
Member

/lgtm
/approve

@kmesh-bot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

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

The pull request process is described here

Details 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

@kmesh-bot kmesh-bot merged commit baca1ea into kmesh-net:main May 23, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Format markdown files in make format

5 participants