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

Minor typos #7497

Closed
NathanBaulch opened this issue Aug 9, 2024 · 7 comments · Fixed by #7487
Closed

Minor typos #7497

NathanBaulch opened this issue Aug 9, 2024 · 7 comments · Fixed by #7487
Assignees

Comments

@NathanBaulch
Copy link
Contributor

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 grpc/grpc-proto#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
Copy link
Contributor

@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
Copy link
Contributor Author

NathanBaulch commented Aug 12, 2024

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

  • cancelation (6)
  • ceritificate (6)
  • andcleanup (5)
  • configurated (4)
  • existant (3)
  • indaequate (3)
  • unimplemneted (3)
  • aggretator (2)
  • certifcate (2)
  • fo (2)
  • keealive (2)
  • readd (2)
  • remainining (2)
  • shoud (2)
  • shuting (2)
  • sugned (2)
  • unexpect (2)
  • verfies (2)
  • adressses (1)
  • affectes (1)
  • aother (1)
  • asynchorous (1)
  • beld (1)
  • brackers (1)
  • bufer (1)
  • calle (1)
  • catapul (1)
  • closd (1)
  • communitcated (1)
  • configurtion (1)
  • congiguration (1)
  • containning (1)
  • contetion (1)
  • deaclock (1)
  • depenedency (1)
  • effet (1)
  • entris (1)
  • failes (1)
  • flakyness (1)
  • forwarrd (1)
  • gerenated (1)
  • grpah graph (1)
  • handerler (1)
  • hte (1)
  • indiacte (1)
  • indiciating (1)
  • insenitive (1)
  • ithe (1)
  • leakcheker (1)
  • limtit (1)
  • metedata (1)
  • nesponse (1)
  • ninetyninth (1)
  • ocnfiguration (1)
  • onepassed one (1)
  • ot (1)
  • polcies (1)
  • polcy (1)
  • potentional (1)
  • prinicipal (1)
  • prinicpal (1)
  • recusively (1)
  • repoting (1)
  • resemebling to (1)
  • respsectively (1)
  • routeguild (1)
  • sceanrio (1)
  • serch (1)
  • sice (1)
  • simillar (1)
  • specfied (1)
  • statusmakes (1)
  • successul (1)
  • tesing (1)
  • testsdata (1)
  • throtlled (1)
  • tranport (1)
  • transpot (1)
  • unrary (1)
  • unsupport (1)
  • utiliztion (1)
  • walkaround (1)
  • wathers (1)

@NathanBaulch
Copy link
Contributor Author

better to send one pr word as per your list

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

@purnesh42H
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Member

dfawley commented Aug 13, 2024

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants