Skip to content

Conversation

@RodneyU215
Copy link
Contributor

Summary

Fixing #481 by properly passing oauth params with aiohttp.BasicAuth.

Fixing #436 by returning the entire SlackResponse in SlackApiError's.

Requirements (place an x in each [ ])

@RodneyU215 RodneyU215 added Priority: High bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 2x labels Oct 5, 2019
@RodneyU215 RodneyU215 requested a review from seratch October 5, 2019 04:26
@RodneyU215 RodneyU215 self-assigned this Oct 5, 2019
@codecov
Copy link

codecov bot commented Oct 5, 2019

Codecov Report

Merging #527 into master will decrease coverage by 0.02%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #527      +/-   ##
==========================================
- Coverage   68.98%   68.96%   -0.03%     
==========================================
  Files          15       15              
  Lines        1709     1711       +2     
  Branches       96       97       +1     
==========================================
+ Hits         1179     1180       +1     
  Misses        507      507              
- Partials       23       24       +1
Impacted Files Coverage Δ
slack/errors.py 100% <ø> (ø) ⬆️
slack/web/client.py 33.61% <0%> (ø) ⬆️
slack/web/slack_response.py 100% <100%> (+2.22%) ⬆️
slack/web/base_client.py 79.61% <33.33%> (-1.58%) ⬇️

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 70883cb...436eb96. Read the comment docs.

@RodneyU215
Copy link
Contributor Author

@seratch can you take a look at these fixes?

Copy link
Contributor

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Both look good to me. 👍

@tyndyll
Copy link

tyndyll commented Oct 7, 2019

Will this be merged soon? This blocks all OAuth access via the SDK

@RodneyU215 RodneyU215 merged commit 612c68d into master Oct 8, 2019
@RodneyU215
Copy link
Contributor Author

Thanks @seratch!

@tyndyll I'll work on getting a patch release out today for this.

@RodneyU215 RodneyU215 deleted the RU_fixing_oauth_and_response_data branch October 8, 2019 12:53
@RodneyU215 RodneyU215 mentioned this pull request Oct 8, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 2x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants