-
Notifications
You must be signed in to change notification settings - Fork 22.4k
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
chore: Expand MarkdownLint CI paths #7146
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed whitespace fixups
files/en-us/web/javascript/javascript_technologies_overview/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/javascript/reference/errors/invalid_for-of_initializer/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/javascript/reference/errors/invalid_for-of_initializer/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/javascript/reference/global_objects/atomics/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/javascript/reference/global_objects/sharedarraybuffer/index.md
Outdated
Show resolved
Hide resolved
I assume this is linting the new markdown docs - @wbamberg would you be able to take a look, it all seems fine to me but it would be good to get another pair of eyes. This changes |
Thanks for this PR @nschonni ! Could you please add a description or a comment explaining what it's doing? |
Sorry, I guess I missed that when I opened it. |
Could add a problem matcher to add inline comments to the PR here, or add it later |
Thanks @nschonni ! I will take a look at this but it might take me a little time :). |
@nschonni Do you know someone with Admin privileges to take care of the failing check in the meantime? |
https://github.com/mdn/content/blob/main/REVIEWING.md#topic-review-owners says
|
So @Rumyra might know, who that would be for dev (instead of content). |
I'll wait for @wbamberg to have a closer look, then check this out 👍 |
Ping @wbamberg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR, and sorry to be so slow looking at it. I've been deep in some thickets of ancient HTML.
- as far as I can tell this works just fine to enable markdownlint for mdn/content
- I think enabling markdownlint for mdn/content is a good idea
- the rules seem sensible and compatible with the converted Markdown we have (which is post-Prettier). I couldn't find any enabled rules which I disagreed with
- I had a few questions, which are all about rules I'd love us to be able to enable. But I understand we might not want to fix up the content yet. Maybe we should file some issues like "Enable markdownlint MD001"?
I think this will need some documentation in the project README so contributors don't get frustrated by build failures.
I'd also like to hear wider feedback from people not just about the specific rules to enforce but about our general policy for which CI tools we should be running for mdn/content.
@@ -0,0 +1,22 @@ | |||
{ | |||
"default": true, | |||
"MD001": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great to enable this one. How many failures do we get with it enabled?
This rule is triggered when you skip heading levels in a markdown document
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
files/en-us/web/javascript/guide/details_of_the_object_model/index.md:136 MD001/heading-increment/header-increment Heading levels should only increment by one level at a time
[Expected: h3; Actual: h4]
files/en-us/web/javascript/guide/using_promises/index.md:306 MD001/heading-increment/header-increment Heading levels should only increment by one level at a time [Expected: h3; Actual: h4]
files/en-us/web/javascript/memory_management/index.md:192 MD001/heading-increment/header-increment Heading levels should only increment by one level at a time [Expected: h3; Actual: h4]
files/en-us/web/javascript/reference/functions/rest_parameters/index.md:52 MD001/heading-increment/header-increment Heading levels should only increment by one level at a time [Expected: h3; Actual: h4]
files/en-us/web/javascript/reference/global_objects/object/create/index.md:159 MD001/heading-increment/header-increment Heading levels should only increment by one level at a
time [Expected: h3; Actual: h4]
"MD010": false, | ||
"MD013": false, | ||
"MD024": false, | ||
"MD026": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to enable this one IMO:
This rule is triggered on any heading that has one of the specified normal or full-width punctuation characters as the last character in the line
I wonder how many failures it would give us :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
files/en-us/web/javascript/data_structures/index.md:129:43 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: '!']
files/en-us/web/javascript/reference/functions/rest_parameters/index.md:200:65 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: '.']
files/en-us/web/javascript/reference/global_objects/eval/index.md:95:20 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: '!']
"MD036": false, | ||
"MD037": false, | ||
"MD040": false, | ||
"MD042": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would also be great to enable this one:
This rule is triggered when an empty link is encountered:
[an empty link]()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly just needs some escaping of the square backets
files/en-us/web/javascript/reference/global_objects/array/index.md:359:60 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/bigint64array/index.md:100:61 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/biguint64array/index.md:100:62 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/float32array/index.md:98:60 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/float64array/index.md:98:60 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/int16array/index.md:98:60 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/int8array/index.md:99:57 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/map/index.md:305:44 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@match/index.md:105:61 MD042/no-empty-links No empty links [Context: "[@@replace]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@match/index.md:106:60 MD042/no-empty-links No empty links [Context: "[@@search]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@match/index.md:107:59 MD042/no-empty-links No empty links [Context: "[@@split]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@replace/index.md:120:59 MD042/no-empty-links No empty links [Context: "[@@match]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@replace/index.md:121:60 MD042/no-empty-links No empty links [Context: "[@@search]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@replace/index.md:122:59 MD042/no-empty-links No empty links [Context: "[@@split]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@search/index.md:101:59 MD042/no-empty-links No empty links [Context: "[@@match]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@search/index.md:102:61 MD042/no-empty-links No empty links [Context: "[@@replace]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@search/index.md:103:59 MD042/no-empty-links No empty links [Context: "[@@split]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@split/index.md:104:59 MD042/no-empty-links No empty links [Context: "[@@match]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@split/index.md:105:61 MD042/no-empty-links No empty links [Context: "[@@replace]()"]
files/en-us/web/javascript/reference/global_objects/regexp/@@split/index.md:106:60 MD042/no-empty-links No empty links [Context: "[@@search]()"]
files/en-us/web/javascript/reference/global_objects/regexp/index.md:99:59 MD042/no-empty-links No empty links [Context: "[@@match]()"]
files/en-us/web/javascript/reference/global_objects/regexp/index.md:101:62 MD042/no-empty-links No empty links [Context: "[@@matchAll]()"]
files/en-us/web/javascript/reference/global_objects/regexp/index.md:103:61 MD042/no-empty-links No empty links [Context: "[@@replace]()"]
files/en-us/web/javascript/reference/global_objects/regexp/index.md:105:60 MD042/no-empty-links No empty links [Context: "[@@search]()"]
files/en-us/web/javascript/reference/global_objects/regexp/index.md:107:59 MD042/no-empty-links No empty links [Context: "[@@split]()"]
files/en-us/web/javascript/reference/global_objects/set/index.md:61:56 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:24:48 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:25:58 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:26:50 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:27:44 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:28:44 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:94:48 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:95:58 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:96:50 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:97:44 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/iterator/index.md:98:44 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/symbol/match/index.md:60:47 MD042/no-empty-links No empty links [Context: "[@@match]()"]
files/en-us/web/javascript/reference/global_objects/symbol/matchall/index.md:20:134 MD042/no-empty-links No empty links [Context: "[@@matchAll]()"]
files/en-us/web/javascript/reference/global_objects/symbol/matchall/index.md:51:98 MD042/no-empty-links No empty links [Context: "[@@matchAll]()"]
files/en-us/web/javascript/reference/global_objects/symbol/matchall/index.md:65:50 MD042/no-empty-links No empty links [Context: "[@@matchAll]()"]
files/en-us/web/javascript/reference/global_objects/symbol/replace/index.md:16:73 MD042/no-empty-links No empty links [Context: "[@@replace]()"]
files/en-us/web/javascript/reference/global_objects/symbol/replace/index.md:52:49 MD042/no-empty-links No empty links [Context: "[@@replace]()"]
files/en-us/web/javascript/reference/global_objects/symbol/search/index.md:16:72 MD042/no-empty-links No empty links [Context: "[@@search]()"]
files/en-us/web/javascript/reference/global_objects/symbol/search/index.md:52:48 MD042/no-empty-links No empty links [Context: "[@@search]()"]
files/en-us/web/javascript/reference/global_objects/symbol/split/index.md:16:71 MD042/no-empty-links No empty links [Context: "[@@split]()"]
files/en-us/web/javascript/reference/global_objects/symbol/split/index.md:50:47 MD042/no-empty-links No empty links [Context: "[@@split]()"]
files/en-us/web/javascript/reference/global_objects/typedarray/entries/index.md:75:26 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/typedarray/index.md:231:26 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/typedarray/keys/index.md:75:26 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/uint16array/index.md:99:59 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/uint32array/index.md:98:59 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/uint8array/index.md:99:58 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
files/en-us/web/javascript/reference/global_objects/uint8clampedarray/index.md:98:65 MD042/no-empty-links No empty links [Context: "[@@iterator]()"]
"MD037": false, | ||
"MD040": false, | ||
"MD042": false, | ||
"MD045": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would also be great to enable this one:
This rule is triggered when an image is missing alternate text (alt text) information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
files/en-us/web/javascript/guide/details_of_the_object_model/index.md:116 MD045/no-alt-text Images should have alternate text (alt text)
files/en-us/web/javascript/guide/details_of_the_object_model/index.md:244 MD045/no-alt-text Images should have alternate text (alt text)
files/en-us/web/javascript/guide/introduction/index.md:94 MD045/no-alt-text Images should have alternate text (alt text)
files/en-us/web/javascript/reference/global_objects/math/index.md:151 MD045/no-alt-text Images should have alternate text (alt text)
files/en-us/web/javascript/reference/global_objects/promise/index.md:36 MD045/no-alt-text Images should have alternate text (alt text)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the lists of files which need updates for us to enable those rules! I will file issues to enable them, but think we should wait until all the content is Markdownified before we try to enable them. Otherwise it's just another barrier to getting into Markdown.
I do think before we can merge this we should have an easy way for people to run the linter locally, and document that in the contribution docs. We do something similar for BCD (https://github.com/mdn/browser-compat-data/blob/main/docs/testing.md#validate-the-data) and interactive examples (https://github.com/mdn/interactive-examples/blob/master/JS-Example-Style-Guide.md#javascript-coding-style). What do you think?
Yes, those are a little more difficult to automate/fix, but I think the other rules are good to have during the convesion, as they can pick up potential rendering issues as the files are converted |
I kept this isolated to the CI, but there is no reason it couldn't be added to the actual package.json as an extra target. I'm not sure the best place for the documentation though. It seems like most of it is in the README.md currently |
No description provided.