-
-
Notifications
You must be signed in to change notification settings - Fork 585
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
[docs] Add textlint #4905
[docs] Add textlint #4905
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.
This is just amazing, should save you just hours of reviewing time. Thanks!
I note that some of the rules are inflexible and can force unnecessary changes, but we should just accept that.
Download the artifacts for this pull request:
See Testing a PR |
@rfay I found a way to not take forever on this! No change to the spell checker, only a sparing introduction of textlint. Trying to get a simpler GitHub Action working. I have a rule set that rejects unfinished todo items and the only thing failing here is this line from the version history. (I also don’t know why that file uses checkboxes and bullets.) How can I fix this? |
.github/workflows/docscheck.yml
Outdated
- uses: rdohms/textlint-action@latest | ||
with: | ||
rulesets: 'textlint-filter-rule-comments textlint-rule-no-todo textlint-rule-stop-words textlint-rule-terminology' | ||
path: '{README.md,version-history.md,docs/**}' |
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.
IMO version-history.md is not something that needs to be checked at all. Just meant extra work for you in this. It's not included in docs because it corrupts the search so much.
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.
I don’t mind since it’s both public-facing and important, and I figure it should be held to the same standard as writing in the documentation. (Same with the developer docs this also fixes—though the spell checker would go wild there if we stopped ignoring that section.)
Are you saying you’d like me to remove that from the check? The fixing-up is already done so after this PR it’s only a matter of maintenance/conformance.
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 checking the box. The other option would have been to remove the line, which would have been completely and historically correct.
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.
Whoops! I was going for correctness—removed the line instead. The fact that we’re discussing style and accuracy is a reason to include this file, IMO!
The only ones that felt noisy to me were cases of “built in” (vs. “built-in”) that were legitimately correct—but rewriting strengthened the sentences anyway. You can change the rule sets, and there are projects like alex that can really shake things up and improve them—but I think the important part is to keep them consistent for public-facing text. |
Easiest way is to exclude version-history.md. Second easiest for unfinished checkboxes is to check the box, since it eventually got done. (Not sure why that was listed there because it was not accomplished at that time. (Powers that be in drud wanted it to require a NDA of participants, so I balked and put it off until drud went away.) Third easiest is to allow unchecked checkboxes. |
@rfay I can’t tell if I managed to break actual important tests or if they’re just having a bad day, but the textlint GitHub Action works now and this is ready for review. |
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.
Awesome, thanks so much! That was a lot less than a week!
WSL2 Docker Desktop is mostly broken since latest release. I'll look for other things.
If you’re bored, try setting up Vale and throw its |
As usual, merge if/when you’re comfortable with this. |
Thanks! I'll probably let it finish most tests since it touches a variety of things in case anything has gone wrong. |
The Issue
Nobody wants to read a writing style guide.
How This PR Solves The Issue
This introduces the same textlint rule set from the ddev.com project, including a GitHub Actions check, and fixes most of the errors.
Manual Testing Instructions
This project doesn’t use a
package.json
file so it’s not a quicknpm install
, but you can install textlint and its rulesets globally like it suggests in the Makefile:Then run
textlint {README.md,version-history.md,docs/**}
ormake textlint
and have a wondrous time tidying up. (The VS Code extension works well!)Automated Testing Overview
It tests public-facing plain text in
README.md
,version-history.md
, and everything in thedocs/
directory.Related Issue Link(s)
Release/Deployment Notes
Won’t mangle DDEV builds or deployment, but I have no idea if the GitHub Action works yet so we’ll be finding out together.