Spelling #9778

jsoref · 2020-10-11T22:05:55Z

This PR corrects misspellings identified by the check-spelling action.

The misspellings have been reported at jsoref@6819186#commitcomment-43154265

The action reports that the changes in this PR would make it happy: jsoref@d52634d

Note: this PR does not include the action. If you're interested in running a spell check on every PR and push, that can be offered separately.

Spelling errors make code harder to read, they lead to various other kinds of bugs, they make it harder to search for tokens, ...
I can file an issue for this if requested.

Proposed Changes

spelling

Release Note

NONE

knative-prow-robot · 2020-10-11T22:06:07Z

Welcome @jsoref! It looks like this is your first PR to knative/serving 🎉

knative-prow-robot · 2020-10-11T22:06:07Z

Hi @jsoref. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jsoref

Note: Spelling corrections were mostly picked automatically by Google Sheets, it's quite possible I've messed up, but I try to review before submitting...

jsoref · 2020-10-11T22:07:54Z

config/core/configmaps/observability.yaml

@@ -83,7 +83,7 @@ data:
    logging.request-log-template: '{"httpRequest": {"requestMethod": "{{.Request.Method}}", "requestUrl": "{{js .Request.RequestURI}}", "requestSize": "{{.Request.ContentLength}}", "status": {{.Response.Code}}, "responseSize": "{{.Response.Size}}", "userAgent": "{{js .Request.UserAgent}}", "remoteIp": "{{js .Request.RemoteAddr}}", "serverIp": "{{.Revision.PodIP}}", "referer": "{{js .Request.Referer}}", "latency": "{{.Response.Latency}}s", "protocol": "{{.Request.Proto}}"}, "traceId": "{{index .Request.Header "X-B3-Traceid"}}"}'

    # If true, the request logging will be enabled.
-    # NB: up to and including Knative version 0.18 if logging.requst-log-template is non-empty, this value


Google says that this is not present beyond one gist

jsoref · 2020-10-11T22:08:19Z

pkg/activator/net/revision_backends.go

@@ -109,7 +109,7 @@ type revisionWatcher struct {
 	logger        *zap.SugaredLogger

 	// podsAddressable will be set to false if we cannot
-	// probe a pod directly, but its cluster IP has beeen successfully probed.


Sometimes people intentionally add extra letters, this doesn't appear to be such an instance

jsoref · 2020-10-11T22:09:10Z

pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/reconciler.go

@@ -179,7 +179,7 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error {
 	logger := logging.FromContext(ctx)

 	// Initialize the reconciler state. This will convert the namespace/name
-	// string into a distinct namespace and name, determin if this instance of
+	// string into a distinct namespace and name, determine if this instance of


It wouldn't shock me if I'm supposed to change this elsewhere, just let me know where and I should be able to eventually.

@tcnghia: there's a vendor/ directory in this repository, often those are directories that people aren't supposed to change directly, it has instances with determin -- can I just change them directly? Or is there an appropriate place to change it. Here's what my local clones have:
https://gist.github.com/jsoref/6115a4ce35488ecc6797ec3cb79563b4

don't change vendor directory, it gets auto populated with some tooling we have. is a mix of internal and external modules

jsoref · 2020-10-11T22:09:48Z

pkg/reconciler/accessor/networking/certificate.go

@@ -44,7 +44,7 @@ func ReconcileCertificate(ctx context.Context, owner kmeta.Accessor, desired *v1

 	recorder := controller.GetEventRecorder(ctx)
 	if recorder == nil {
-		return nil, fmt.Errorf("recoder for reconciling Certificate %s/%s is not created", desired.Namespace, desired.Name)
+		return nil, fmt.Errorf("recorder for reconciling Certificate %s/%s is not created", desired.Namespace, desired.Name)


I'm always worried about changes like his, but as the line above is recorder, I'm relatively confident here

pkg/reconciler/route/table_test.go

pkg/reconciler/serverlessservice/resources/services_test.go

jsoref · 2020-10-11T22:16:16Z

test/e2e/autoscale_test.go

-		// We want exactly 2. Not 1, not panicing 3, just 2.
+		// We want exactly 2. Not 1, not panicking 3, just 2.


I understand that the verb is panic. The English conjugation is panicking. afaik, panicing isn't commonly (ngrams, trends) used.

this is more like (panic)ing as in go's panic.

Yeah, I get that.

https://www.google.com/search?q=go+%22panicing%22
~298,000 results
+Did you mean: go "panicking"

https://www.google.com/search?q=go+%22panicking%22
~14,300,000 results

this looks good panicking

tcnghia · 2020-10-11T22:25:26Z

/ok-to-test

tcnghia · 2020-10-11T22:26:43Z

@jsoref very nice through update, thanks. Can you please rerun hack/update-codegen.sh (see https://github.com/knative/serving/pull/9778/checks?check_run_id=1239381808 )

jsoref · 2020-10-11T22:34:27Z

@tcnghia: I don't think I have a working go install. I'll have to look later. (I'm happy to give write bits to my fork too...)

tcnghia · 2020-10-11T22:58:39Z

@tcnghia: I don't think I have a working go install. I'll have to look later. (I'm happy to give write bits to my fork too...)

@n3wscott @mattmoor I wonder if this is something we can automate so that our less-frequent contributors can just do /update-codegen and get a commit :) -- or a downloadable patch to use with git apply

mattmoor · 2020-10-13T04:13:21Z

cc @n3wscott

I was just using his library 😉

n3wscott · 2020-10-13T04:32:17Z

We never have supported bash 3, only bash 4

Signed-off-by: Josh Soref <[email protected]>

csantanapr · 2020-10-16T15:54:46Z

Very nice job @jsoref 👍

csantanapr · 2020-10-16T15:56:36Z

config/core/configmaps/autoscaler.yaml

@@ -20,7 +20,7 @@ metadata:
  labels:
    serving.knative.dev/release: devel
  annotations:
-    knative.dev/example-checksum: "12baeac1"
+    knative.dev/example-checksum: "1d830d9e"


remove this change, nothing to do with the spell checking

csantanapr · 2020-10-16T15:56:45Z

cmd/queue/main_test.go

@@ -149,7 +149,7 @@ func TestQueueTraceSpans(t *testing.T) {
 				h := queue.ProxyHandler(breaker, network.NewRequestStats(time.Now()), true /*tracingEnabled*/, proxy)
 				h(writer, req)
 			} else {
-				h := health.ProbeHandler(healthState, tc.prober, true /* isAggresive*/, true /*tracingEnabled*/, nil)
+				h := health.ProbeHandler(healthState, tc.prober, true /* isAggressive*/, true /*tracingEnabled*/, nil)


Oh, I should point out that the first commented item has a leading space (but no trailing), whereas the second has no whitespace. I suspect the latter is the preferred style, but I'm not eager to make changes :-)

csantanapr · 2020-10-16T15:56:57Z

config/core/configmaps/autoscaler.yaml

@@ -153,11 +153,11 @@ data:
    # Scale to zero pod retention period defines the minimum amount
    # of time the last pod will remain after Autoscaler has decided to
    # scale to zero.
-    # This flag is for the situations where the pod starup is very expensive
+    # This flag is for the situations where the pod startup is very expensive


csantanapr · 2020-10-16T15:59:31Z

test/e2e/autoscale_test.go

-		// We want exactly 2. Not 1, not panicing 3, just 2.
+		// We want exactly 2. Not 1, not panicking 3, just 2.


this looks good panicking

csantanapr · 2020-10-16T16:01:00Z

config/core/configmaps/observability.yaml

@@ -20,7 +20,7 @@ metadata:
  labels:
    serving.knative.dev/release: devel
  annotations:
-    knative.dev/example-checksum: "3af7fc85"
+    knative.dev/example-checksum: "97c1d10b"


I wonder why your tool changing yaml checking sums

csantanapr · 2020-10-16T16:06:45Z

@jsoref the PR looks good, just a few comments
I would remove the checksum changes (2 places), don't why they are there unless I'm unaware that they are required

knative.dev/example-checksum

csantanapr · 2020-10-16T16:08:41Z

ignore my comments to checksum I saw @julz comment

Hi @jsoref - looks like you'll need to run ./hack/update-checksums.sh following the rebase to update the checksums again

csantanapr · 2020-10-16T16:08:49Z

/approve

markusthoemmes

Thanks for bearing with us through this and thanks for everybodz helping to unblock things 🎉

/lgtm
/approve

knative-prow-robot · 2020-10-16T17:22:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csantanapr, jsoref, markusthoemmes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [markusthoemmes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 11, 2020

knative-prow-robot requested review from coryrc, JRBANCEL, julz, markusthoemmes, mdemirhan, nak3, shashwathi, vagababov, whaught and yanweiguo October 11, 2020 22:06

knative-prow-robot added area/API API objects and controllers area/autoscale labels Oct 11, 2020

knative-prow-robot added area/networking needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Oct 11, 2020

google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Oct 11, 2020

jsoref commented Oct 11, 2020

View reviewed changes

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 11, 2020

This comment has been minimized.

Sign in to view

jsoref mentioned this pull request Oct 13, 2020

hack/update-codegen.sh is broken on macOS 10.14.6 (Mojave) #9792

Closed

jsoref added 14 commits October 15, 2020 09:35

spelling: request

42f8b88

Signed-off-by: Josh Soref <[email protected]>

spelling: return

e29e9f1

Signed-off-by: Josh Soref <[email protected]>

spelling: requests

c45caae

Signed-off-by: Josh Soref <[email protected]>

spelling: revision

cd4bf62

Signed-off-by: Josh Soref <[email protected]>

spelling: stdin

bb79a05

Signed-off-by: Josh Soref <[email protected]>

spelling: sidecar

6591895

Signed-off-by: Josh Soref <[email protected]>

spelling: stabilize

acb53e7

Signed-off-by: Josh Soref <[email protected]>

spelling: stabilized

2d89607

Signed-off-by: Josh Soref <[email protected]>

spelling: startup

f4082ae

Signed-off-by: Josh Soref <[email protected]>

spelling: statmessage

81d0ce2

Signed-off-by: Josh Soref <[email protected]>

spelling: verifying

bcc1356

Signed-off-by: Josh Soref <[email protected]>

spelling: windermere

880da9f

Signed-off-by: Josh Soref <[email protected]>

spelling: windows

46507ad

Signed-off-by: Josh Soref <[email protected]>

chore: update hashes

6bbd0a3

Signed-off-by: Josh Soref <[email protected]>

jsoref force-pushed the spelling branch from f7d9294 to 6bbd0a3 Compare October 15, 2020 13:35

csantanapr reviewed Oct 16, 2020

View reviewed changes

markusthoemmes approved these changes Oct 16, 2020

View reviewed changes

knative-prow-robot assigned markusthoemmes Oct 16, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 16, 2020

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 16, 2020

knative-prow-robot merged commit 1c8ec8f into knative:master Oct 16, 2020

jsoref deleted the spelling branch October 16, 2020 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spelling #9778

Spelling #9778

jsoref commented Oct 11, 2020 •

edited

Loading

knative-prow-robot commented Oct 11, 2020

knative-prow-robot commented Oct 11, 2020

jsoref left a comment

jsoref Oct 11, 2020

jsoref Oct 11, 2020

jsoref Oct 11, 2020

jsoref Oct 12, 2020

csantanapr Oct 16, 2020

jsoref Oct 11, 2020

jsoref Oct 11, 2020

vagababov Oct 12, 2020

jsoref Oct 12, 2020

csantanapr Oct 16, 2020

tcnghia commented Oct 11, 2020

tcnghia commented Oct 11, 2020

jsoref commented Oct 11, 2020

tcnghia commented Oct 11, 2020 •

edited

Loading

This comment has been minimized.

mattmoor commented Oct 13, 2020

n3wscott commented Oct 13, 2020

csantanapr commented Oct 16, 2020

csantanapr Oct 16, 2020

csantanapr Oct 16, 2020

jsoref Oct 17, 2020

csantanapr Oct 16, 2020

csantanapr Oct 16, 2020

csantanapr Oct 16, 2020

csantanapr commented Oct 16, 2020

csantanapr commented Oct 16, 2020

csantanapr commented Oct 16, 2020

markusthoemmes left a comment

knative-prow-robot commented Oct 16, 2020

		// We want exactly 2. Not 1, not panicing 3, just 2.
		// We want exactly 2. Not 1, not panicking 3, just 2.

Spelling #9778

Spelling #9778

Conversation

jsoref commented Oct 11, 2020 • edited Loading

Proposed Changes

knative-prow-robot commented Oct 11, 2020

knative-prow-robot commented Oct 11, 2020

jsoref left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tcnghia commented Oct 11, 2020

tcnghia commented Oct 11, 2020

jsoref commented Oct 11, 2020

tcnghia commented Oct 11, 2020 • edited Loading

This comment has been minimized.

mattmoor commented Oct 13, 2020

n3wscott commented Oct 13, 2020

csantanapr commented Oct 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csantanapr commented Oct 16, 2020

csantanapr commented Oct 16, 2020

csantanapr commented Oct 16, 2020

markusthoemmes left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Oct 16, 2020

jsoref commented Oct 11, 2020 •

edited

Loading

tcnghia commented Oct 11, 2020 •

edited

Loading