grpc / grpc-go

The Go language implementation of gRPC. HTTP/2 based RPC
https://grpc.io
Apache License 2.0
21.07k stars 4.38k forks source link

Minor typos #7497

Closed NathanBaulch closed 2 months ago

NathanBaulch commented 3 months ago

PR #7487 fixes a bunch of typos across the entire project, however it's been deemed too far reaching for a single commit from an unknown author without prior discussion. Understandable.

How should these changes be broken up? cc @purnesh42H

I admit that I didn't take the time to figure out how to install the correct protoc + plugin versions to regenerate all code (https://buf.build takes care of that in my projects) so I wouldn't be surprised if that caused some problems.

FWIW a couple of these typos have already been fixed upstream via https://github.com/grpc/grpc-proto/pull/158.

For anyone curious (cc @arvindbr8), I used a combination of the excellent typos tool along with the "Typos" IntelliJ IDE inspection. Both methods produce a non-trivial number of false positives that have to be manually checked however.

purnesh42H commented 2 months ago

@NathanBaulch i the PR you have a list of words that are spelled wrong. Can you mention that list in the issue? Also, better to send one pr word as per your list. Thanks

NathanBaulch commented 2 months ago

Sure, here's the full list with number of occurrences:

NathanBaulch commented 2 months ago

better to send one pr word as per your list

I don't follow, can you clarify this request @purnesh42H?

purnesh42H commented 2 months ago

@NathanBaulch scratch that. Initially i was thinking we can have one pr to fix each word typo but that will be too many PRs. Can you send smaller PR that doesn't modify more than 2-3 files? If the code change is in generated interfaces, that's ok as that require each interface to be generated again

NathanBaulch commented 2 months ago

I don't believe I touched any code generated files (assuming you mean *.pb.go), so 89 files in groups of 3 means I need to break it into 30 PRs?

dfawley commented 2 months ago

It seems the tests are failing due to a reflection test (caused by the typo fix?), otherwise, let's just rip off the bandaid and merge the big PR in its current state.

NathanBaulch commented 2 months ago

Fixed @dfawley - I've restored a couple of intentional typos in the reflection tests and everything is passing now.