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.

Sign up for GitHub

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

Jump to bottom

chore: Fix typos #1108

Merged
merged 3 commits into from
Sep 23, 2024
Merged

chore: Fix typos #1108

merged 3 commits into from
Sep 23, 2024

Conversation

NathanBaulch
Copy link
Contributor

Just thought I'd contribute some typo fixes that I stumbled on. Nothing controversial (hopefully).

Use the following command to get a quick and dirty summary of the specific corrections made:

git diff HEAD^! --word-diff-regex='\w+' -U0 \
  | grep -E '\[\-.*\-\]\{\+.*\+\}' \
  | sed -r 's/.*\[\-(.*)\-\]\{\+(.*)\+\}.*/\1 \2/' \
  | sort | uniq -c | sort -n

FWIW, the top typos are:

  • nsted (20)
  • emtpy (13)
  • detatch (6)
  • unparseable (4)
  • specficied (2)
  • straming (2)
  • inteegration (1)
  • mutliple (1)
  • modifer (1)

@NathanBaulch NathanBaulch changed the title Fix typos chore: Fix typos Sep 20, 2024
stephenh
stephenh approved these changes Sep 22, 2024
View reviewed changes
Copy link
Owner

@stephenh stephenh left a comment

Choose a reason for hiding this comment

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

Hi @NathanBaulch , thanks for the PR! The typo fixes are appreciated. Just one ask to revert some of the comments in code generated from *.proto files that we don't control/can't fix the source typo in, but otherwise this is great.

integration/meta-typings-as-const/google/protobuf/descriptor.ts Outdated
@@ -167,7 +167,7 @@ export interface FieldDescriptorProto {
* If true, this is a proto3 "optional". When a proto3 field is optional, it
* tracks presence regardless of field type.
*
* When proto3_optional is true, this field must be belong to a oneof to
Copy link
Owner

Choose a reason for hiding this comment

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

@NathanBaulch these google/*/*.ts files/comments come from comments within the google *.proto files that we can't change, so I think we need to revert any descriptor.ts changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

protos/google/protobuf/compiler/plugin.proto Outdated
@@ -103,7 +103,7 @@ message CodeGeneratorResponse {
// This should be used to indicate errors in .proto files which prevent the
// code generator from generating correct code. Errors which indicate a
// problem in protoc itself -- such as the input CodeGeneratorRequest being
// unparseable -- should be reported by writing a message to stderr and
// unparsable -- should be reported by writing a message to stderr and
Copy link
Owner

Choose a reason for hiding this comment

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

These comments are also probably coming from a google *.proto file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also done!

@NathanBaulch
Copy link
Contributor Author

Restored all typos in **/google/protobuf/**/*.* files, as requested.

@stephenh
Copy link
Owner

Looks great, thanks @NathanBaulch !

@stephenh stephenh merged commit ffacf3a into stephenh:main Sep 23, 2024
6 checks passed
@NathanBaulch NathanBaulch deleted the typos branch September 23, 2024 20:39
@stephenh
Copy link
Owner

🎉 This issue has been resolved in version 2.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephenh stephenh stephenh approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NathanBaulch @stephenh