Skip to content

Conversation

@seratch
Copy link
Contributor

@seratch seratch commented Aug 27, 2022

As we discussed at #713, with the current implementation, req.context.client can be a singleton object in an App instance. This can be a cause of unexpected behaviors when a developer modify the properties such as token of the instance on the fly. This pull request improve the internals to be more robust even for such use cases.

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

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Document pages under /docs
  • Others

Requirements (place an x in each [ ])

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • 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 ./scripts/install_all_and_run_tests.sh after making the changes.

@codecov
Copy link

codecov bot commented Aug 27, 2022

Codecov Report

Merging #714 (b0a6576) into main (9df884e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head b0a6576 differs from pull request most recent head 9bcbff8. Consider uploading reports for the commit 9bcbff8 to get more accurate results

@@            Coverage Diff             @@
##             main     #714      +/-   ##
==========================================
- Coverage   92.05%   92.05%   -0.01%     
==========================================
  Files         172      172              
  Lines        5869     5865       -4     
==========================================
- Hits         5403     5399       -4     
  Misses        466      466              
Impacted Files Coverage Δ
slack_bolt/app/app.py 87.21% <100.00%> (-0.06%) ⬇️
slack_bolt/app/async_app.py 94.17% <100.00%> (-0.03%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

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.

3 participants