Skip to content
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: Fix WireGuard spelling #16293

Merged
merged 1 commit into from May 25, 2021

Conversation

gandro
Copy link
Member

@gandro gandro commented May 25, 2021

According to WireGuard's Trademark Usage Policy [1], WireGuard must be
written with a capital W and a capital G.

[1] https://www.wireguard.com/trademark-policy/

@gandro gandro added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.10 labels May 25, 2021
@gandro gandro requested a review from a team May 25, 2021 10:13
@gandro gandro requested a review from a team as a code owner May 25, 2021 10:13
@gandro gandro requested review from kkourt and qmonnet May 25, 2021 10:13
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks good, but I wonder if we have a way to prevent adding Wireguard again in the future. The spell file currently contains Wireguard, I'm curious to see if it complains on your changes. But even if it does, we are likely to change it anyway to have wireguard (all lowercase) in #16238, and this will result of Sphinx spellchecks being case insensitive :/.

@qmonnet
Copy link
Member

qmonnet commented May 25, 2021

Experimenting with Sphinx's spell-check: The current Wireguard shouldn't trigger any complaint on your changes, because it seems to be case-insensitive when there is no upper case or when only the first letter is upper case.

Would you consider to change the spelling file to WireGuard for now, even if we change back in a future PR? Would seem cleaner to me. Ok you just did while I was writing, thanks!

@gandro
Copy link
Member Author

gandro commented May 25, 2021

PR looks good, but I wonder if we have a way to prevent adding Wireguard again in the future. The spell file currently contains Wireguard, I'm curious to see if it complains on your changes. But even if it does, we are likely to change it anyway to have wireguard (all lowercase) in #16238, and this will result of Sphinx spellchecks being case insensitive :/.

Good question, I don't have a good answer either. I at least updated the spelling in the dictionary. It's also complicated by the fact that we still have some uses of "Wireguard" in the cilium status command output, so a simple grep lint would still need to deal with exceptions.

@qmonnet
Copy link
Member

qmonnet commented May 25, 2021

we still have some uses of "Wireguard" in the cilium status command output

Should these be updated, too?

@qmonnet
Copy link
Member

qmonnet commented May 25, 2021

(Looking at the docs we can maybe implement some custom filter to allow either wireguard (for Helm values) or WireGuard, but reject Wireguard.)

@gandro
Copy link
Member Author

gandro commented May 25, 2021

we still have some uses of "Wireguard" in the cilium status command output

Should these be updated, too?

Probably, yes. But I want to do that in a separate PR, as it is a code change and I want to be sure to test that separately.

@gandro
Copy link
Member Author

gandro commented May 25, 2021

(Looking at the docs we can maybe implement some custom filter to allow either wireguard (for Helm values) or WireGuard, but reject Wireguard.)

So interestingly enough, now that I fixed the spelling in the dictionary, it seems to actually consider upper/lowercase (because I missed one):

If the words are not misspelled, run:
Documentation/update-spelling_wordlist.sh Wireguard 

According to WireGuard's Trademark Usage Policy [1], WireGuard must be
written with a capital W and a capital G.

[1] https://www.wireguard.com/trademark-policy/

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/gandro/docs-fix-wg-spelling branch from bb535eb to 4c7c557 Compare May 25, 2021 11:39
@gandro gandro added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 25, 2021
@borkmann borkmann merged commit 5d2e72f into cilium:master May 25, 2021
@qmonnet
Copy link
Member

qmonnet commented May 25, 2021

So interestingly enough, now that I fixed the spelling in the dictionary, it seems to actually consider upper/lowercase

My understanding (from playing with it a little) is:

  • wireguard -> case insensitive
  • Wireguard -> “probably wireguard with a capital because it was at the beginning of a sentence” -> case insensitive
  • WireGuard -> capital elsewhere than first letter only -> case sensitive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants