-
Notifications
You must be signed in to change notification settings - Fork 72
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
Change function signature from rest.API to rest.Send. #19
Change function signature from rest.API to rest.Send. #19
Conversation
HI @janczer, So that we don't have a breaking change, can you please implement With Best Regards, Elmer |
891a214
to
bc35a0f
Compare
Sure. I added it and smash to the one commit. |
rest_test.go
Outdated
@@ -95,7 +95,7 @@ func TestRest(t *testing.T) { | |||
Headers: Headers, | |||
QueryParams: queryParams, | |||
} | |||
response, e := API(request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add these tests back in, in addition to the new ones you created for Send
. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added function testingAPI
, and I'm sending to this function like parameter functions Send
and API
because code the same.
bc35a0f
to
84c163f
Compare
My apologies @janczer, Could you please update the merge conflicts? Thanks! With Best Regards, Elmer |
84c163f
to
e6cf9c6
Compare
I rebased my branche and merge conflicts. Thanks! |
Hello @janczer, |
I think you are also missing the API() passthrough on the Client struct, or sendgrid-go also needs a PR: |
Yes, sendgrid-go will need an updated PR. That would be a most excellent hacktoberfest contribution ;) |
Honestly, I do not appreciate breaking the package API. |
Thanks for catching this @jerbob92! |
According to #13.