[tools] Add markdownlint to the pre-commit #2717
Conversation
Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
stdlib/docs/development.md
Outdated
| @@ -263,7 +263,7 @@ ls **/*.mojo | entr sh -c "./scripts/build-stdlib.sh && mojo main.mojo" | |||
| Now, every time you save a Mojo file, it packages the standard library and | |||
| runs `main.mojo`. | |||
|
|
|||
| **Note**: you should stop `entr` while doing commits, otherwise you could have | |||
| __Note__: you should stop `entr` while doing commits, otherwise you could have | |||
There was a problem hiding this comment.
Question Is this change because of differences in the markdownlint-cli version we use internally, or something else? Looks like I'm on 0.34.0 locally.
There was a problem hiding this comment.
That may be possible I can try it out. Give me a sec
There was a problem hiding this comment.
I tried with 0.34.0 and I still got
stdlib/docs/development.md:266:1 MD050/strong-style Strong style should be consistent [Expected: underscore; Actual: asterisk]
Looking at the documentation it makes sense: https://github.com/DavidAnson/markdownlint/blob/main/doc/md050.md
By default markdownlint just wants all emphasis to be the same. Being __something__ or **something**. I don't know what the tool installed on your machine is doing but I would say that the pre-commit is right here.
If you want we can add some configuration to enforce one style or the other, that would avoid any blurry line concerning this rule. What do you think?
EDIT: Added the rule to always use asterisk to avoid confusion.
There was a problem hiding this comment.
I see, that makes sense. I don't feel strongly on the version, but I'd just like our internal and external to be consistent with the style they pick to avoid drift. @modularml/docs do you have opinions here?
Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
|
@JoeLoser done. We now always force |
|
!sync |
JoeLoser
added
the
imported-internally
|
✅🟣 This contribution has been merged 🟣✅ Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours. We use Copybara to merge external contributions, click here to learn more. |
modularbot
added
the
merged-internally
|
I just landed this internally, but our copybara isn't managing .pre-commit-config.yaml, ditto with the workflow files etc. We should probably have it manage the pre commit config yaml file for easy (thoughts @patrickdoc?). The net effect is the config and md file change landed, and once those propagate in the nightly tonight, then we can send a different smaller PR for the pre commit config file itself. Or is it easier to just merge this wholesale into the nightly branch now, @patrickdoc? There shouldn't be any conflicts in theory during the auto rebase, but didn't want to cause any issues. |
[External] [tools] Add markdownlint to the pre-commit Fix #2716 This pre-commit hook will also fix easy mistakes :) It also doesn't require any manual installation. It just works™ The CI will trigger an error if a markdown file does not conform to the rules of markdownlint Example of pre-commit failure in the CI: https://github.com/modularml/mojo/actions/runs/9132279705/job/25113289498?pr=2663 Co-authored-by: Gabriel de Marmiesse <gabriel.demarmiesse@datadoghq.com> Closes #2717 MODULAR_ORIG_COMMIT_REV_ID: 56e9f0d2e5498625762effab6e7c0d83b6c8b560
|
Landed in b6b01b4! Thank you for your contribution 🎉 |
Split off from modularml#2717 since Copybara isn't currently managing the `.pre-commit-config.yaml` file. The other changes from modularml#2717 landed already in modularml@b6b01b4 Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
[External] [tools] Add markdownlint to the pre-commit Fix modularml#2716 This pre-commit hook will also fix easy mistakes :) It also doesn't require any manual installation. It just works™ The CI will trigger an error if a markdown file does not conform to the rules of markdownlint Example of pre-commit failure in the CI: https://github.com/modularml/mojo/actions/runs/9132279705/job/25113289498?pr=2663 Co-authored-by: Gabriel de Marmiesse <gabriel.demarmiesse@datadoghq.com> Closes modularml#2717 MODULAR_ORIG_COMMIT_REV_ID: 56e9f0d2e5498625762effab6e7c0d83b6c8b560
Split off from modularml#2717 since Copybara isn't currently managing the `.pre-commit-config.yaml` file. The other changes from modularml#2717 landed already in modularml@b6b01b4 Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
Fix #2716
This pre-commit hook will also fix easy mistakes :) It also doesn't require any manual installation. It just works™
The CI will trigger an error if a markdown file does not conform to the rules of markdownlint
Example of pre-commit failure in the CI: https://github.com/modularml/mojo/actions/runs/9132279705/job/25113289498?pr=2663