Skip to content

Conversation

@fwump38
Copy link

@fwump38 fwump38 commented Oct 17, 2020

Summary

Fixes #851 (at least for the main branch. This change needs to be applied to v3 branch in slack and slack_sdk respective locations)

Category (place an x in each of the [ ])

  • slack.web.WebClient (Web API client)
  • slack.webhook.WebhookClient (Incoming Webhook, response_url sender)
  • slack.web.classes (UI component builders)
  • slack.rtm.RTMClient (RTM client)
  • Documents
  • Others

Requirements (place an x in each [ ])

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run python setup.py validate after making the changes.

@gitwave gitwave bot added the untriaged label Oct 17, 2020
@codecov
Copy link

codecov bot commented Oct 17, 2020

Codecov Report

Merging #852 into main will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #852   +/-   ##
=======================================
  Coverage   87.02%   87.02%           
=======================================
  Files          30       30           
  Lines        3800     3800           
  Branches      341      341           
=======================================
  Hits         3307     3307           
  Misses        344      344           
  Partials      149      149           
Impacted Files Coverage Δ
slack/web/classes/blocks.py 93.77% <100.00%> (ø)

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 aa2e9b2...a439b5b. Read the comment docs.

@fwump38
Copy link
Author

fwump38 commented Oct 17, 2020

Per this comment I tried running:

python setup.py codegen
python setup.py validate

But got errors for both. Also, per the PR, I'm not sure how to apply these changes to the other branch(es) without making a new PR

@seratch seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 2x Version: 3x and removed untriaged labels Oct 18, 2020
@seratch seratch added this to the 2.9.3 milestone Oct 18, 2020
@seratch seratch self-assigned this Oct 18, 2020
@seratch
Copy link
Contributor

seratch commented Oct 18, 2020

But got errors for both. Also, per the PR, I'm not sure how to apply these changes to the other branch(es) without making a new PR

@fwump38 Is it possible to share the errors you've encountered? If it's [Errno 54] Connection reset by peer in tests, it's a known issue: #808 Regarding the codegen command, it exists only in v3 branch, so you don't need to run the command. The validate command does the same as part of it.

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.

LGTM

@seratch
Copy link
Contributor

seratch commented Oct 18, 2020

Thanks for fixing this. I will apply the same change to v3 branch.

@seratch seratch merged commit 0495601 into slackapi:main Oct 18, 2020
@seratch seratch mentioned this pull request Oct 18, 2020
12 tasks
@fwump38
Copy link
Author

fwump38 commented Oct 20, 2020

The errors I got when running python setup.py validate were user error. I was using the system python instead of a virtualenv python so a bunch of tests were failing. I don't recall what python version they were failing on but they all pass now.

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 Version: 3x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HeaderBlock text missing default_type

2 participants