-
Notifications
You must be signed in to change notification settings - Fork 193
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
WIP: add vale rules for wording #4861
base: main
Are you sure you want to change the base?
Conversation
@pepopowitz Mind taking a peek to see if I have outlined and placed these properly? I'll loop in the tech writers for final 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.
I think I sent you down the incorrect path, with these rules, I'm sorry!
I also would like to see some proof of what it looks like in GitHub when these rules are violated. That could be commits added in this PR, the review comments screenshotted, and then the commits rolled back....or it could be another PR opened against this branch, and linked to from here.
Let me know if I can help!
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.
Should these apply to the Optimize docs as well? Adding them to the styles/camunda/docsInstance
folder means they'll only apply to the main docs instance.
I think you want to move them to the styles/camunda/all
folder instead, so that they apply to both the main docs instance and the Optimize instance.
@@ -0,0 +1,10 @@ | |||
extends: existence |
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.
Unfortunately I can't find anything in the vale docs that explains this very well, but I now realize that I suggested the wrong base rule for these.
If you look at the example here, it extends "substitution" rather than "existence", and defines a "swap" array instead of "tokens". In doing so, it enables the linter to provide a suggestion for the correct value. I think that would give us more actionable feedback in the PR review comments, particularly a commit-able suggestion for the alternative.
I'm a little bit guessing at that, though. We would know for sure if you added some changes to this PR, or opened a PR into this PR with violations.
I see what you're suggesting here @pepopowitz -- WDYT of the updates? |
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.
Yes, exactly! These look great, except two replacements that I think were overlooked.
@camunda/tech-writers Are there any additional terms you would like to add here? I'm trying to keep the first iteration pretty minimal, but if I'm missing something that would be helpful please let me know and I can add to this PR before merging 👍 |
Configmap: ConfigMap | ||
config map: ConfigMap | ||
Config Map: ConfigMap | ||
Docker compose: Docker Compose |
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 add "docker compose", as in the command (all lowercase)?
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.
Or if that's written intentionally in code, as a terminal command, will this get flagged?
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 will get flagged -- similar to my lack of "javascript" currently. There are a few threads online discussing how we can ignore code blocks, but I may open this as a subsequent issue so I can do some research and testing on what that looks like?
Description
Adds Vale linting warnings for non-inclusive language and terms to avoid. Closes #1904.
When should this change go live?
hold
label or convert to draft PR)PR Checklist
/versioned_docs
directory./docs
directory (aka/next/
).