-
Notifications
You must be signed in to change notification settings - Fork 8.1k
vale: add some acronyms and relax validation #20726
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
Conversation
GHSA is for GitHub Security Advisories; https://docs.github.com/en/code-security/security-advisories/working-with-global-security-advisories-from-the-github-advisory-database/about-the-github-advisory-database Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- DSOS: Docker-Sponsored Open Source (DSOS) program - DVP: Docker Verified Publisher (DVP) - DCT: Docker Content-Trust Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[vale] reported by reviewdog 🐶
[Docker.Acronyms] 'TTY' has no definition.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| Debootstrap | ||
| Dev Environments? | ||
| Django | ||
| docker/ |
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.
Hope this one works; looks like this file accepts some RegEx? Not sure how it matches things (so if it would need something like [^]]+ after it?
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.
Do we need some exception for docker-XXXX as well? (thinking 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.
I think the only valid use for docker-compose would be in a code span, when referring to the legacy command, so in that case it wouldn't be flagged by vale anyway.
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 think this works, at least it doesn't seem to in my editor
| docker/ | |
| docker/ |
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.
LOL what's your suggested change above? I don't see what it ... changed 🥹
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.
Try this
diff --git a/.github/vale/Docker/Capitalization.yml b/.github/vale/Docker/Capitalization.yml
new file mode 100644
index 0000000000..6d95064783
--- /dev/null
+++ b/.github/vale/Docker/Capitalization.yml
@@ -0,0 +1,10 @@
+extends: existence
+message: "Please capitalize Docker."
+level: error
+ignorecase: false
+action:
+ name: replace
+ params:
+ - Docker
+tokens:
+ - 'docker(?!/cli)'
diff --git a/.github/vale/config/vocabularies/Docker/accept.txt b/.github/vale/config/vocabularies/Docker/accept.txt
index 78f3e66f8c..c461e2f7bf 100644
--- a/.github/vale/config/vocabularies/Docker/accept.txt
+++ b/.github/vale/config/vocabularies/Docker/accept.txt
@@ -35,7 +35,6 @@ Ddosify
Debootstrap
Dev Environments?
Django
-Docker
Docker Build Cloud
Docker Business
Docker Dasboard| params: | ||
| - Docker | ||
| tokens: | ||
| - 'docker(?!/cli)' |
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 guess we may need buildx, compose, <other repos> as well; should those be added as separate items in this list, or do we want to try capturing them in a single RegEx?
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.
ya you're right, we should. Maybe this?
| - 'docker(?!/cli)' | |
| - 'docker(?!/[a-z-]+)' |
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.
Yup; something like that seems good (but make sure to test indeed) 🙈
I updated with your suggestion
Try to prevent false-positives on links to GitHub issues, e.g.;
[docker/docker-ce-packaging#1050](docker/docker-ce-packaging#1050)
Check failure on line 93 in content/engine/release-notes/27.md
[vale] reported by reviewdog 🐶
[Vale.Terms] Use 'Docker' instead of 'docker'.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Description
vale: add GHSA to acronyms
GHSA is for GitHub Security Advisories;
https://docs.github.com/en/code-security/security-advisories/working-with-global-security-advisories-from-the-github-advisory-database/about-the-github-advisory-database
vale: add DCT, DSOS and DVP to acronyms
vale: add TTY to acronyms
vale: allow 'docker/' in link-captions
Try to prevent false-positives on links to GitHub issues, e.g.;
Related issues or tickets
Reviews