-
Notifications
You must be signed in to change notification settings - Fork 681
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
Spelling #6747
base: main
Are you sure you want to change the base?
Spelling #6747
Conversation
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Hi @jsoref! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace |
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.
As noted in Slack, I'm a human, and I manually selected the corrections. That means I can make mistakes. I didn't spot any for this PR while composing this review, but that doesn't mean there aren't any (I did spot one yesterday for a similar PR that I made elsewhere -- I picked the wrong correction for schame
).
I compose changes in a word-family/correction per commit which means if you object to a correction, I can easily drop it (e.g. into
, or preexisting
). Changes are generally done using tooling for some flavor of search-and-replace (although sometimes they're purely manual). I use VS Code and test w/ CI to try to ensure that I haven't broken tests (the tests here required tags which I didn't have initially, so I had to borrow a tag...). I try to ensure that I don't break whitespace (review shows that in at least one place I adjusted whitespace -- I'm not sure if that was me or VS Code, I think it was me being careful to ensure the replacement included a space).
It's possible for me to split by directory or file extension, but I generally try to discourage that, and while it's occasionally possible for me to split by "comment" vs "code", I wouldn't be able to do that for this repository this year.
#### MacOS | ||
#### macOS |
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.
brand
@@ -187,7 +187,7 @@ func getIndex(condType string, subconds []SubCondition) int { | |||
return -1 | |||
} | |||
|
|||
// GetConditionFor returns the a pointer to the condition for a given type, | |||
// GetConditionFor returns a pointer to the condition for a given type, |
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.
It's hard to pick the proper article, but there can be only one...
@@ -27,11 +27,11 @@ import ( | |||
|
|||
func TestUpstreamTLSTransportSocket(t *testing.T) { | |||
tests := map[string]struct { | |||
ctxt *envoy_transport_socket_tls_v3.UpstreamTlsContext | |||
ctx *envoy_transport_socket_tls_v3.UpstreamTlsContext |
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.
there are many possible ways to abbreviate context, but this short form dominates this repository (and happens to be my preference)
@@ -171,7 +171,7 @@ func TestSDSShouldNotIncrementVersionNumberForUnrelatedSecret(t *testing.T) { | |||
|
|||
// issue 1169, an invalid certificate should not be | |||
// presented by SDS even if referenced by an ingress object. | |||
func TestSDSshouldNotPublishInvalidSecret(t *testing.T) { | |||
func TestSDSShouldNotPublishInvalidSecret(t *testing.T) { |
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.
It's certainly fine to drop this, but a good tool/human will be able to properly parse this as:
Test SDS Should Not Publish Invalid Secret
Just as they should be able to properly parse HTTPProxy as HTTP Proxy (as long as it's properly spelled...) or TestUpstreamTLSWithHTTPRoute...
@@ -462,7 +462,7 @@ func TestBackendTLSPolicyPrecedenceOverUpstreamProtocolAnnotationWithHTTPRoute(t | |||
|
|||
// Test that a unique cluster name is generated when there is an HTTPProxy with upstream TLS settings | |||
// and an HTTPRoute with a BackendTLSPolicy, configured with unique TLS settings, targeting the same service. | |||
func TestUpstreamTLSWithHTTPRouteANDHTTPProxy(t *testing.T) { | |||
func TestUpstreamTLSWithHTTPRouteAndHTTPProxy(t *testing.T) { |
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.
Afaict, this was not an acronym...
@@ -1917,7 +1917,7 @@ <h3 id="projectcontour.io/v1.HeaderMatchCondition">HeaderMatchCondition | |||
</td> | |||
<td> | |||
<p>Name is the name of the header to match against. Name is required. | |||
Header names are case insensitive.</p> | |||
Header names are case-insensitive.</p> |
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 should call out that I don't have the 1.31 doc fixes here because I branched before that. If you're doing a running review, hopefully you're telling github which changes you've seen so that I can add in the changes for 1.31 at some point. My preference is to amend things into the corresponding commits, but I'm happy to do w/ unsquashed "fixup" commits (and not force-push) if that helps reviewers...
The way I tend to do catch ups is to rebase the spell-check + spelling branches onto a fetched upstream and repeat the workflow until I get back to 0 errors (but I do it in a separate repository to avoid people seeing until I'm ready)...
@@ -454,7 +454,7 @@ spec: | |||
strategy: RequestHash | |||
requestHashPolicies: | |||
- queryParameterHashOptions: | |||
prameterName: param1 | |||
parameterName: param1 |
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.
As noted earlier for a similar documentation item, this typo only appears in docs and there's nothing to indicate it's intentional.
@@ -31,7 +31,7 @@ footer { | |||
p { | |||
margin: 0px; | |||
} | |||
.copywrite { | |||
.copyright { |
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 a style change -- the only consumer in this repository is also changed.
it's possible for someone to include this css file in an external consumer (as it's technically a public resource, and thus it's techncially an api change), but, they really shouldn't, ...
@@ -11,7 +11,7 @@ | |||
</div> | |||
</div> | |||
<div class="bottom-links"> | |||
<div class="copywrite"> | |||
<div class="copyright"> |
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 the corresponding consumer
@@ -14,7 +14,7 @@ if CURRENT_TAG=$(git describe --tags --exact-match 2>/dev/null); then | |||
git tag -l --sort=-v:refname | grep -A5 -x $CURRENT_TAG | grep -v -x $CURRENT_TAG | grep -v 'alpha\|beta\|rc' | head -n 1 | |||
elif git describe --tags --abbrev=0 | grep -q -v v1.2.0; then | |||
# Note: Contour v1.2.0 was improperly tagged on main so we | |||
# ignore it to ensure we dont hit that case here. | |||
# ignore it to ensure we don't hit that case here. |
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.
while this seems fairly innocuous, it's the kind of change that should be scrutinized the most.
thankfully, #
will properly ignore '
s and thus I believe it's harmless (but you shoukd never trust a random PR submitter).
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.
Rather than have this as a one-off, sounds like this is a suggestion to use the https://github.com/marketplace/actions/check-spelling action rather than codespell?
If you want to swap that action out or fix the codespell configuration to catch these errors we would be open to that change
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6747 +/- ##
==========================================
- Coverage 81.76% 81.04% -0.73%
==========================================
Files 133 133
Lines 15944 20001 +4057
==========================================
+ Hits 13037 16210 +3173
- Misses 2614 3498 +884
Partials 293 293
|
This PR corrects misspellings identified by the check-spelling action.
The misspellings have been reported at https://github.com/jsoref/contour/actions/runs/11628797911#summary-32384590478
The action reports that the changes in this PR would make it happy: https://github.com/jsoref/contour/actions/runs/11628798577#summary-32384592098