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 Issue channel access token API #63

Merged
merged 5 commits into from
Jun 25, 2019
Merged

Conversation

drillbits
Copy link
Contributor

@sugyan
Copy link
Contributor

sugyan commented Jul 20, 2018

Sorry for the late response.
We were carefully considering whether to include these methods in this SDK.
I tried issuing a pull-request to follow the latest changes, so please check it. 🙇

drillbits#1

linebot/oauth.go Outdated

res, err := call.c.postform(call.ctx, APIEndpointIssueAccessToken, body)
if res != nil && res.Body != nil {
defer res.Body.Close()
Copy link
Contributor

@linxGnu linxGnu Apr 25, 2019

Choose a reason for hiding this comment

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

Please follow practice:

if err != nil {
    return nil, err
}
defer closeResponse(res)

This practice prevent malform payload response from server. And make client connection reusable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review. I fixed it!

@drillbits drillbits force-pushed the oauth-accessToken branch from d0acad4 to 36f9842 Compare May 2, 2019 04:35
@drillbits
Copy link
Contributor Author

@sugyan
Sorry for the late response, too...
I merged drillbits#1 and synced with the upstream.

@codecov-io
Copy link

codecov-io commented May 2, 2019

Codecov Report

Merging #63 into master will increase coverage by 0.2%.
The diff coverage is 79.59%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #63     +/-   ##
=========================================
+ Coverage   72.11%   72.31%   +0.2%     
=========================================
  Files          23       24      +1     
  Lines        1757     1806     +49     
=========================================
+ Hits         1267     1306     +39     
- Misses        413      418      +5     
- Partials       77       82      +5
Impacted Files Coverage Δ
linebot/response.go 66.93% <50%> (-1.17%) ⬇️
linebot/client.go 73.75% <66.66%> (-0.58%) ⬇️
linebot/oauth.go 88.57% <88.57%> (ø)

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 e039956...13e4bb1. Read the comment docs.

@sugyan sugyan self-requested a review May 4, 2019 08:56
Copy link
Contributor

@sugyan sugyan left a comment

Choose a reason for hiding this comment

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

LGTM 🙆‍♂

@k2wanko k2wanko self-requested a review May 6, 2019 23:43
@tkgauri
Copy link
Contributor

tkgauri commented May 16, 2019

#154

Copy link
Contributor

@k2wanko k2wanko left a comment

Choose a reason for hiding this comment

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

LGTM

@k2wanko k2wanko merged commit a3e3584 into line:master Jun 25, 2019
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.

7 participants