Skip to content

Use multiple markdownlint.yaml /versions and /schemas #115

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.

Sign up for GitHub

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

Closed
wants to merge 9 commits into from
+46 −5
Closed

Use multiple markdownlint.yaml /versions and /schemas #115

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1a37157
Use a different config for each folder
Bellangelo Dec 31, 2024
df825ab
Add commands for different markdown link configs
Bellangelo Dec 31, 2024
2b02ca7
Update workflows
Bellangelo Dec 31, 2024
b333c2d
Change script to require a config parameter
Bellangelo Jan 1, 2025
1437f02
Add new commands for formatting the different mds
Bellangelo Jan 1, 2025
bb0951b
Group commands under the same namespace
Bellangelo Jan 1, 2025
278d08e
Command was using the old scripts
Bellangelo Jan 1, 2025
b5876e3
Update workflow based on the renaming of the commands
Bellangelo Jan 1, 2025
f9dbf2b
Command was using the old scripts
Bellangelo Jan 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/validate-markdown.yaml
View file
Original file line number Diff line number Diff line change
@@ -26,6 +26,6 @@ jobs:
with:
node-version: '20.x'
- name: Validate markdown
run: npx --yes mdv versions/*.md
run: "npm run markdown:validate"
- name: Lint markdown
run: npx --yes markdownlint-cli --config .markdownlint.yaml versions/*.md
run: "npm run markdown:lint"
8 changes: 7 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -40,7 +40,13 @@
],
"scripts": {
"build": "bash ./scripts/md2html/build.sh",
"format-markdown": "bash ./scripts/format-markdown.sh ./versions/*.md",
"markdown:format:versions": "bash ./scripts/format-markdown.sh --config ./versions/.markdownlint.yaml ./versions/*.md",
"markdown:format:schemas": "bash ./scripts/format-markdown.sh --config ./schemas/.markdownlint.yaml ./schemas/**/*.md",
"markdown:format": "npm run markdown:format:schemas && npm run markdown:format:versions",
"markdown:lint:schemas": "npx --yes markdownlint-cli --config ./schemas/.markdownlint.yaml ./schemas/**/*.md",
"markdown:lint:versions": "npx --yes markdownlint-cli --config ./versions/.markdownlint.yaml ./versions/*.md",
"markdown:lint": "npm run markdown:lint:schemas && npm run markdown:lint:versions",
"markdown:validate": "npx --yes mdv ./{versions,schemas/**}/*.md",
"test": "c8 --100 vitest --watch=false && bash scripts/schema-test-coverage.sh"
}
}
14 changes: 14 additions & 0 deletions schemas/.markdownlint.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Unordered list indentation
MD007:
indent: 2

MD012: false # allow blank lines

MD013:
line_length: 800
tables: false

MD024: false # duplicate headings
MD032: false # blanks-around-lists Lists should be surrounded by blank lines
MD033: false # inline HTML
MD047: false # single-trailing-newline Files should end with a single newline character
Comment on lines +11 to +14
Copy link
Member Author

Choose a reason for hiding this comment

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

Rules 32 and 47 are disabled because they didn't pass. We can enable them again and format the file if that is what we desire.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please use the same rules for all *.md files and change the files that don't follow the rules yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I agree that the same rules apply for specification documents and our internal project documentation in markdown files. I think we already rejected a pull request that applied the same standards to both - We need to be strict and careful with the documents that are published, I'm not sure the same applies if people are adding different content types.

Copy link
Member Author

Choose a reason for hiding this comment

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

HI @ralfhandl , I am not against in what you are proposing but @lornajane is right about the rejected PR. Please check #105

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the commented-out rules are trivial to observe, I'd use the same rules.

The more interesting case will be when we make the rules for new spec documents stricter and will need different rule sets for files in the versions folder, as we already do for OAS.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ralfhandl Ok. But what this means for the other comments that you made? Such as #115 (comment)

To be more precise, are we keeping the multiple lint configs or are we using a single one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer keeping a single rule file and adjusting all Markdown files across all folders accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would relax the rules for non-spec markdown files, we know we need to tighten them for the spec ones (in the version folder, plus the work-in-progress file). There is no need to be fussy about blank lines around lists in files such as CONTRIBUTING which might serve as a barrier to participation - the only requirement of most of our markdown files is that they render sensibly in GitHub (we might even permit GitHub-specific markup in this context). For the specification files, these need to be much more carefully structured and be suitable for rendering with respec, so I would expect a lot more rules there.

25 changes: 23 additions & 2 deletions scripts/format-markdown.sh
View file
Original file line number Diff line number Diff line change
@@ -2,7 +2,28 @@

SRCDIR="$(dirname "${BASH_SOURCE[0]}")" # check on Windows

for filename in $*; do
CONFIG=""

while [[ $# -gt 0 ]]; do
case $1 in
--config)
CONFIG="$2"
shift # Remove --config from processing
shift # Remove the value from processing
;;
*)
FILES+="$1 "
shift # Move to the next argument
;;
esac
done

if [ -z "$CONFIG" ]; then
echo "Error: --config parameter is required."
exit 1
fi

for filename in $FILES; do
Comment on lines +5 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessary, same rules for all files

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this is used for but I definitely don't see that we need this script, can someone explain what the problem was that this was solving? Let's try again!

# mostly to format code blocks with examples, unfortunately messes up bullet lists and tables
npx prettier --write --single-quote $filename

@@ -11,5 +32,5 @@ for filename in $*; do
sed -E -e "s/ +\|/ |/g" -e "s/\| +/| /g" -e "s/-----+/----/g" $filename > $filename.tmp && mv $filename.tmp $filename

# repair the bullet lists and various other markdown formatting issues
npx --yes markdownlint-cli --fix --config $SRCDIR/../.markdownlint.yaml $filename
npx --yes markdownlint-cli --fix --config "$CONFIG" $filename
done
0 .markdownlint.yaml → versions/.markdownlint.yaml
View file
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move back to root folder

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd advocate for locating a config file with a stricter set of rules here, with the all-files rules at the root of the project. We need to use the strict rules for src/overlay.md file on the development branches as well. However I don't see any markdown files in the schemas/ directory so I'm not sure why we have put configuration there.

File renamed without changes.