Vonage / vonage-ruby-sdk

Vonage REST API client for Ruby. API support for SMS, Voice, Text-to-Speech, Numbers, Verify (2FA) and more.
https://developer.vonage.com
Apache License 2.0
217 stars 108 forks source link

Add Patch HTTP method as option in Logger.log_request_info type sig #237

Closed superchilled closed 2 years ago

superchilled commented 2 years ago

Reason for this PR

Adds support for PATCH requests to be passed to the Logger#log_request_info method

What this PR does

codecov-commenter commented 2 years ago

Codecov Report

Merging #237 (468c05b) into dev (e4e4f79) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##              dev     #237   +/-   ##
=======================================
  Coverage   97.69%   97.69%           
=======================================
  Files          70       70           
  Lines        1388     1388           
=======================================
  Hits         1356     1356           
  Misses         32       32           
Impacted Files Coverage Δ
lib/vonage/logger.rb 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

superchilled commented 2 years ago

Is this an optional parameter? Or is it a breaking change?

Not an optional param or a breaking change. Basically updating the type signature to allow Net::HTTP::Patch objects to be passed into the method invocation. Prior to this change you could only pass in Net::HTTP::Post, Net::HTTP::Get, Net::HTTP::Delete, or Net::HTTP::Put objects.

SMadani commented 2 years ago

Is this an optional parameter? Or is it a breaking change?

Not an optional param or a breaking change. Basically updating the type signature to allow Net::HTTP::Patch objects to be passed into the method invocation. Prior to this change you could only pass in Net::HTTP::Post, Net::HTTP::Get, Net::HTTP::Delete, or Net::HTTP::Put objects.

Ohh I think I get it. You're saying the restrictions on types. I forget Ruby isn't strongly typed so you have to do this using some alternative way that I'm not familiar with (is it a comment? Annotation? Is it enforced by the compiler?)

superchilled commented 2 years ago

Is this an optional parameter? Or is it a breaking change?

Not an optional param or a breaking change. Basically updating the type signature to allow Net::HTTP::Patch objects to be passed into the method invocation. Prior to this change you could only pass in Net::HTTP::Post, Net::HTTP::Get, Net::HTTP::Delete, or Net::HTTP::Put objects.

Ohh I think I get it. You're saying the restrictions on types. I forget Ruby isn't strongly typed so you have to do this using some alternative way that I'm not familiar with (is it a comment? Annotation? Is it enforced by the compiler?)

It's a typing library called Sorbet. It basically lets you add type annotations to method definitions. The block ({...}) following the sig invocation (just above the method definition) defines the type signature for that method. The params part defines the parameters and their types -- here there is one param (request) and the use of T.any defines a union type (a type made up of other types), essentially here it is saying that the type of object passed in as request can be any one of those types passed to T.any. The .void bit at the end I guess is kind of the same as it would be in Java; it's basically saying here that the method has no useful return (i.e. we're not going to use the return value for anything, so we don't really care what it is). I say kind of, because the method still has a return value, we're just not concerned with it at the type level (in Ruby, all methods have a return value, even if it's implicit).