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

Add sorbet types to some modules #180

Merged
merged 4 commits into from Oct 16, 2020

Conversation

ohbarye
Copy link
Contributor

@ohbarye ohbarye commented Oct 12, 2020

@bencgreenberg
Thanks for giving me a chance to contribute.
I'm so new to Sorbet, please let me know if this change has anything wrong.

Issue

Part of #178

Changes

Added Sorbet typings to the modules below.

  • Vonage::UserAgent
  • Vonage::GSM7

@hummusonrails
Copy link
Contributor

Hey @ohbarye, so glad to see your PR on this! I'll take a look this week. Thanks for contributing!

lib/vonage/gsm7.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@hummusonrails hummusonrails left a comment

Choose a reason for hiding this comment

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

This looks great, @ohbarye! I've left just a couple small suggestions to incorporate.

ohbarye and others added 2 commits October 15, 2020 22:57
Vonage#180 (comment)

Co-authored-by: Ben Greenberg <13892277+bencgreenberg@users.noreply.github.com>
Vonage#180 (comment)

Co-authored-by: Ben Greenberg <13892277+bencgreenberg@users.noreply.github.com>
@ohbarye
Copy link
Contributor Author

ohbarye commented Oct 15, 2020

@bencgreenberg Thanks for your review, I've addressed them.

@hummusonrails
Copy link
Contributor

LGTM 👍

@hummusonrails hummusonrails self-requested a review October 16, 2020 12:36
@hummusonrails hummusonrails merged commit dcf58c2 into Vonage:master Oct 16, 2020
@ohbarye ohbarye deleted the add-sorbet-types-to-some-modules branch October 16, 2020 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants