-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add Markdown file linter #6895
Add Markdown file linter #6895
Conversation
This adds `check/markdownlint` and a corresponding configuration file `.markdownlint.yaml`. Running `./check/markdownlint` will check all `.md` files recursively from the top of the source tree, ignoring subdirectories whose names start with a `.`. Arguments can be passed to the script so that, e.g., `./check/markdownlint somefilename` will lint just _somefilename_.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6895 +/- ##
=======================================
Coverage 97.86% 97.86%
=======================================
Files 1084 1084
Lines 94408 94404 -4
=======================================
Hits 92393 92393
+ Misses 2015 2011 -4 ☔ View full report in Codecov by Sentry. |
Many existing Cirq Markdown files use bare URLs for things like email addresses, because that's supported by GitHub-flavored Markdown.
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.
Can we rename the config file to .markdownlintrc
?
According to the docs that configuration file would be found by markdownlint from any subdirectory; we could then do without the check/markdownlint script.
Otherwise LGTM.
We can change the config file, but the reason there's a check/markdownlint script is to give it a default argument if the user invokes it simply as If this difference in behavior (compared to other check/ scripts) is okay, then we don't need check/markdownlint. |
I feel that is the better option - I'd prefer to move on to a standard invocation of code checking tools and remove check/sometool scripts as much as possible (#6800). Would you mind adding a soft markdown CI check in a follow-up PR? |
OK, no problem.
Yes – in fact, I had that in mind, and this is a step on that path! |
Per discussion on PR quantumlib#6895, due to an apparent difference in behavior of markdownlint, we decided to switch to using .markdownlintrc.
Per discussion on PR quantumlib#6895, we decided to work towards removing unnecessary check/foo scripts in favor of having people run commands directly.
Woops, @pavoljuhas I just realized that the linter with the behavior I described above is not markdownlint, but yamllint. Still, deleting the check script for the other reasons discussed. I'll do the same for the yamllint PR. |
Per discussion on PR quantumlib#6895, we decided to work towards removing unnecessary check/foo scripts in favor of having people run commands directly.
This adds a config file
.markdownlintrc
for use withmarkdownlint
.