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

Increase file level typedness at least typed: true #183

Merged
merged 4 commits into from
Oct 28, 2020

Conversation

ohbarye
Copy link
Contributor

@ohbarye ohbarye commented Oct 17, 2020

Issue

Part of #178

Changes

As described in https://sorbet.org/docs/metrics, Sorbet recommends us to increase file-level typedness to measure adoption of Sorbet.

So I increased file-level typedness of files that have had low level typedness (typed: ignore or typed: false). With this change, all files get to have at least typed: true.

```
lib/vonage/namespace.rb:64: Dynamic constant references are unsupported https://srb.help/5001
    64 |      unless type::REQUEST_HAS_BODY || params.empty?
                     ^^^^^^^^^^^^^^^^^^^^^^

lib/vonage/namespace.rb:80: Dynamic constant references are unsupported https://srb.help/5001
    80 |      self.class.request_body.update(message, params) if type::REQUEST_HAS_BODY
                                                                 ^^^^^^^^^^^^^^^^^^^^^^
Errors: 2
```
The code utilizes `method_missing` to dispatch the method call
dynamically.

```
lib/vonage/numbers/response.rb:6: Method error_code does not exist on Vonage::Numbers::Response https://srb.help/7003
     6 |    error_code == '200'
            ^^^^^^^^^^
Errors: 1
```

For this kind of cases, Sorbet provides an escape hatch

https://sorbet.org/docs/troubleshooting#tunsafeself
@ohbarye ohbarye marked this pull request as ready for review October 17, 2020 03:05
@codecov-io
Copy link

codecov-io commented Oct 17, 2020

Codecov Report

Merging #183 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #183   +/-   ##
=======================================
  Coverage   99.65%   99.65%           
=======================================
  Files          54       54           
  Lines         862      862           
=======================================
  Hits          859      859           
  Misses          3        3           
Impacted Files Coverage Δ
lib/vonage/abstract_authentication.rb 100.00% <ø> (ø)
lib/vonage/applications/list_response.rb 100.00% <ø> (ø)
lib/vonage/basic.rb 100.00% <ø> (ø)
lib/vonage/bearer_token.rb 100.00% <ø> (ø)
lib/vonage/form_data.rb 100.00% <ø> (ø)
lib/vonage/jwt.rb 100.00% <ø> (ø)
lib/vonage/numbers/list_response.rb 100.00% <ø> (ø)
lib/vonage/params.rb 100.00% <ø> (ø)
lib/vonage/secrets/list_response.rb 100.00% <ø> (ø)
lib/vonage/voice/list_response.rb 100.00% <ø> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dcf58c2...cdeb3ef. Read the comment docs.

@hummusonrails
Copy link
Contributor

Another good PR, @ohbarye. I'll take a look this week.

@hummusonrails hummusonrails merged commit 449e1d5 into Vonage:master Oct 28, 2020
@ohbarye ohbarye deleted the increase-file-level-typedness branch October 29, 2020 02:15
@ohbarye
Copy link
Contributor Author

ohbarye commented Oct 29, 2020

Thanks for your review!

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.

3 participants