chore: adopt markdownlint and markdown-table-formatter for *.md by matifali · Pull Request #15831 · coder/coder

matifali · 2024-12-11T15:30:08Z

Removed dependence on prettier in favor of markdownlint and markdown-table-formatter for markdown files.

…docs-md

Co-authored-by: Muhammad Atif Ali <atif@coder.com>

- Replace Prettier with markdownlint for documentation formatting. - Rename `docs-pr.yaml` to `docs-lint.yaml` for clarity. - Adjust `Makefile` to use `fmt/biome` instead of `fmt/prettier`. - Remove `.prettierignore` and `.prettierignore.include` files. - Apply markdownlint formatting across relevant documentation files.

- Restrict triggers to main branch and pull requests - Remove automatic fix option in markdownlint-cli2 command

- Update `setup-node` action to specify pnpm version. - Remove redundant node_modules installation command. - Use specific commit hashes for action versions in workflows. - Adjust paths in Makefile for better file management.

EdwardAngert

amazing- thanks @matifali !

🚀

johnstcn

I note a number of heading changes (e.g. # => ##) -- are we sure this won't break any existing documentation links?

matifali · 2025-01-02T09:26:31Z

I note a number of heading changes (e.g. # => ##) -- are we sure this won't break any existing documentation links?

There is a Markdown rule against having multiple H1 headings in a file. Yes, it will not break Markdown anchors, which are independent of heading levels.
@EdwardAngert can confirm this.

mafredri · 2025-01-02T08:54:20Z

+      - name: lint
+        if: steps.changed-files.outputs.any_changed == 'true'
+        run: |
+          pnpm exec markdownlint-cli2 ${{ steps.changed-files.outputs.all_changed_files }}


This works now but could at some point fail with a shell error: argument list too long. Using xargs is one option and maybe markdown-lint has some other built-in function.

If a file has spaces that would also be problematic.

mafredri · 2025-01-02T09:02:14Z

+        if: steps.changed-files.outputs.any_changed == 'true'
+        run: |
+          # markdown-table-formatter requires a space separated list of files
+          echo ${{ steps.changed-files.outputs.all_changed_files }} | tr ',' '\n' | pnpm exec markdown-table-formatter --check


Same as before, you could just add xargs in front of pnpm here. You could use null instead of newline (tr , ’\0’, xargs -0) for additional safety. For ultimate safety, store files in env via yaml and echo the env in addition to null handling and all bases are covered.

I like this - seems like it might make it easier to debug issues later too

something like a commented out debug line that writes the env contents into a file? is that possible/useful?

I'm not sure what it would specifically look like or if it's a cat or echo (printf?) thing, but something like:

cat <<< "$changed-files" > "list-changed-files.txt"

@mafredri I am leaving the suggestions here out of this PR. I need to learn the risks here and can address them in a follow-up PR. I also welcome you taking a jab at this.

mafredri · 2025-01-02T09:46:42Z

@@ -0,0 +1,18 @@
+{


Does this file support json comment syntax? If yes, might be nice to detail why we change some of these rules. Otherwise it's could be nice to stick to stock settings as close as possible, given that it has sane defaults.

Will add comments and convert to *.jsonc.

mafredri · 2025-01-02T09:49:24Z

@@ -20,7 +20,7 @@
 		"playwright:install": "playwright install --with-deps chromium",
 		"playwright:test": "playwright test --config=e2e/playwright.config.ts",
 		"playwright:test-ui": "playwright test --config=e2e/playwright.config.ts --ui $([[ \"$CODER\" == \"true\" ]] && echo --ui-port=7500 --ui-host=0.0.0.0)",
-		"gen:provisioner": "protoc --plugin=./node_modules/.bin/protoc-gen-ts_proto --ts_proto_out=./e2e/ --ts_proto_opt=outputJsonMethods=false,outputEncodeMethods=encode-no-creation,outputClientImpl=false,nestJs=false,outputPartialMethods=false,fileSuffix=Generated,suffix=hey -I ../provisionersdk/proto ../provisionersdk/proto/provisioner.proto && pnpm exec prettier --ignore-path '/dev/null' --cache --write './e2e/provisionerGenerated.ts'",
+		"gen:provisioner": "protoc --plugin=./node_modules/.bin/protoc-gen-ts_proto --ts_proto_out=./e2e/ --ts_proto_opt=outputJsonMethods=false,outputEncodeMethods=encode-no-creation,outputClientImpl=false,nestJs=false,outputPartialMethods=false,fileSuffix=Generated,suffix=hey -I ../provisionersdk/proto ../provisionersdk/proto/provisioner.proto",


I see prettier was removed here, but no alternative given it its place?

./e2e/provisionerGenerated.ts is a generated file so should not format I guess.
This is already in the ignore list of biome.

coder/site/biome.jsonc

Line 8 in 687b015

"e2e/**/*Generated.ts",

mafredri · 2025-01-02T09:56:49Z

@@ -56,7 +56,7 @@ OPTIONS:



These files must not be changed, probably best to not touch anything in any testdata folder. Although, if they changed as a result of changing the Go files then that's OK.

What is the best way to do that?
These files are generated from code and so does the API and CLI docs. And to make the docs follow the rules this is an artifact.

I will further explore how best to deal with this.

mafredri · 2025-01-02T10:06:02Z

-	WARN = 3,
-	ERROR = 4,
-	UNRECOGNIZED = -1,
+  TRACE = 0,


This file is incorrectly indented. It'll be annoying if a developer is making local changes here and the whole file is reformatted.

EdwardAngert · 2025-01-02T14:48:25Z

@johnstcn

I note a number of heading changes (e.g. # => ##) -- are we sure this won't break any existing documentation links?

@matifali

There is a Markdown rule against having multiple H1 headings in a file. Yes, it will not break Markdown anchors, which are independent of heading levels. @EdwardAngert can confirm this.

+1 to what @matifali says here - totally reasonable question and good catch though. Whatever the number of # in the markdown, they all anchor as a single # in the URL (which is why there's another rule against duplicate headings - it would just link to the first instance). If we have a table of contents on any pages, those might potentially look a little different, but it shouldn't affect anything in terms of navigation

Added path exclusions to resolve issues with table formatting in `api/schema.md` and `api/templates.md`.

bpmct · 2025-01-03T02:59:36Z

wow

matifali · 2025-01-03T13:03:38Z

 			"out/",
 			"test-results/",
 			"e2e/**/*Generated.ts",
-			"src/api/*Generated.ts",


removing as we want to lint/fmt these files.

EdwardAngert and others added 20 commits November 25, 2024 16:51

disable prettier for docs md

fe34c79

make fmt

5d1429a

fix md in jfrog-artifactory

a8b2217

Merge remote-tracking branch 'origin/main' into 216-disable-prettier-…

8df4ea1

…docs-md

add markdownlint-cli2 and rules

819074c

make fmt

150fdef

fix typo

4a1fb29

alt text to contributors

acee64b

Apply suggestions from code review

9109a7e

Co-authored-by: Muhammad Atif Ali <atif@coder.com>

move action; remove output format

a6b8f83

link fixes

6e8aa60

fixes and ignore no-bare-urls

9f14438

md fixes

40a94f1

add changed.yml

43435a5

md lint changed files

1da5f23

remove redundancy from docs-pr.yaml

59a92c9

mdlinter ignore docs/reference

34ca8cb

fix errors

4dc14ea

Merge branch 'main' into 216-disable-prettier-docs-md

b7331df

github-actions Bot assigned matifali Dec 11, 2024

Refactor docs lint workflow in docs-lint.yaml

e9431a6

- Restrict triggers to main branch and pull requests - Remove automatic fix option in markdownlint-cli2 command

matifali requested a review from EdwardAngert December 11, 2024 15:34

matifali added 2 commits December 11, 2024 15:36

Update docs lint workflow to include site directory

2f84e0f

Add packageManager to site package.json

e3ac0b1

matifali marked this pull request as draft December 11, 2024 15:47

matifali removed the request for review from EdwardAngert December 11, 2024 15:47

matifali force-pushed the atif/bye-bye-prettier branch 2 times, most recently from 3d065a5 to 4cd592b Compare December 11, 2024 15:52

Refactor actions and workflows for consistency

9ccfcfd

- Update `setup-node` action to specify pnpm version. - Remove redundant node_modules installation command. - Use specific commit hashes for action versions in workflows. - Adjust paths in Makefile for better file management.

matifali force-pushed the atif/bye-bye-prettier branch from cd5fcdc to 0d41834 Compare December 31, 2024 12:51

EdwardAngert approved these changes Dec 31, 2024

View reviewed changes

EdwardAngert added the docs Area: coder.com/docs label Dec 31, 2024

Merge branch 'main' into atif/bye-bye-prettier

f97ce22

matifali requested a review from mafredri January 2, 2025 04:54

johnstcn reviewed Jan 2, 2025

View reviewed changes

mafredri reviewed Jan 2, 2025

View reviewed changes

johnstcn approved these changes Jan 2, 2025

View reviewed changes

matifali added 2 commits January 2, 2025 17:10

wip

687b015

wip 2

fe265eb

matifali force-pushed the atif/bye-bye-prettier branch from c82df5a to fe265eb Compare January 2, 2025 19:11

matifali added 2 commits January 2, 2025 19:15

fixup

33f4b81

Update markdownlint config for schema and templates docs

11dd374

Added path exclusions to resolve issues with table formatting in `api/schema.md` and `api/templates.md`.

matifali added 2 commits January 3, 2025 16:47

Merge branch 'main' into atif/bye-bye-prettier

43f568b

make -B gen

dadc689

matifali enabled auto-merge (squash) January 3, 2025 12:10

matifali disabled auto-merge January 3, 2025 12:13

matifali added 5 commits January 3, 2025 17:14

Delete coderd/database/dbmock/gomock_reflect_2684282611/prog.go

c375423

make -B gen

6b95860

make -B gen

8fefeef

make update-golden-files

8731666

fixup

9f6adaa

matifali commented Jan 3, 2025

View reviewed changes

matifali enabled auto-merge (squash) January 3, 2025 13:05

matifali merged commit 94f5d52 into main Jan 3, 2025

matifali deleted the atif/bye-bye-prettier branch January 3, 2025 13:13

github-actions Bot locked and limited conversation to collaborators Jan 3, 2025

Uh oh!

Conversation

matifali commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EdwardAngert left a comment

Choose a reason for hiding this comment

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

matifali commented Jan 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EdwardAngert Jan 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matifali Jan 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

EdwardAngert commented Jan 2, 2025

Uh oh!

bpmct commented Jan 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

matifali commented Dec 11, 2024 •

edited

Loading

matifali commented Jan 2, 2025 •

edited

Loading

EdwardAngert Jan 2, 2025 •

edited

Loading

matifali Jan 3, 2025 •

edited

Loading