Skip to content

Improve the warning message in App/AsyncApp constructor #256 - #257

Merged
seratch merged 2 commits into
slackapi:mainfrom
seratch:improve-warning-messages
Mar 11, 2021
Merged

seratch merged 2 commits into
slackapi:mainfrom
seratch:improve-warning-messages

Conversation

@seratch

@seratch seratch commented Mar 9, 2021 •

Copy link
Copy Markdown
Contributor

This pull request improves the warning messages in App / AsyncApp constructor, that tells that both SLACK_BOT_TOKEN and authorize/installation_store are unexpectedly enabled. Refer to #256 for the context.

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

codecov Bot commented Mar 9, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #257 (547f3e7) into main (e80a21c) will decrease coverage by 0.09%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #257      +/-   ##
==========================================
- Coverage   91.37%   91.28%   -0.10%     
==========================================
  Files         160      160              
  Lines        4973     4979       +6     
==========================================
+ Hits         4544     4545       +1     
- Misses        429      434       +5     
Impacted Files Coverage Δ
slack_bolt/app/async_app.py 94.18% <0.00%> (-0.56%) ⬇️
slack_bolt/app/app.py 86.44% <33.33%> (-0.45%) ⬇️
slack_bolt/logger/messages.py 86.79% <50.00%> (-1.45%) ⬇️

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 e80a21c...86d8a3b. Read the comment docs.

@misscoded misscoded left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feel free to forgo my suggestions if you disagree or if I've misunderstood the context/intent.

Comment thread slack_bolt/logger/messages.py Outdated
Comment thread slack_bolt/logger/messages.py Outdated
Co-authored-by: Alissa Renz <alissa.renz@gmail.com>
@seratch

seratch commented Mar 10, 2021

Copy link
Copy Markdown
Contributor Author

@misscoded Thanks for the suggestion, all have been merged! I will do more tests before merging this.

@seratch
seratch merged commit dafc833 into slackapi:main Mar 11, 2021
@seratch
seratch deleted the improve-warning-messages branch March 11, 2021 03:46
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.

2 participants