chore: improve type checking behavior#1470
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1470 +/- ##
=======================================
Coverage 91.31% 91.32%
=======================================
Files 229 229
Lines 7266 7270 +4
=======================================
+ Hits 6635 6639 +4
Misses 631 631 ☔ View full report in Codecov by Sentry. |
zimeg
left a comment
zimeg
left a comment
There was a problem hiding this comment.
@WilliamBergamin Another amazing changeset to maintenance 🐍 ✨
I left some question about CI scripts but nothing to block type checking changes from landing 🚢
| if TYPE_CHECKING: | ||
| from slack_bolt.app.async_app import AsyncApp |
There was a problem hiding this comment.
🌟 praise: Love to find circular imports avoided without a sacrifice to sureness!
| pip install -r requirements/async.txt | ||
| pip install -r requirements/adapter.txt | ||
| - name: Type check all modules | ||
| run: mypy --config-file pyproject.toml |
There was a problem hiding this comment.
📺 question: Are we wanting to split this from scripts moving forward?
👾 ramble: I find bash scripts can be most portable but these workflows seem to make use of ordered dependencies?
There was a problem hiding this comment.
Yess this aims to take advantage of ordered dependencies, ensuring that "synchronous" Bolt will never depend on "asynchronous" dependencies
I would like to keep the bash scripts in sync but I think this has some benefit to ensure we never accidentally depend on asynchronous dependencies
| port: int | ||
| path: str | ||
| host: str | ||
| bolt_app: "AsyncApp" # type: ignore[name-defined] |
There was a problem hiding this comment.
🪓 praise: These comments have been confusing to me. Thanks for finding pattern of fixes!
2 participants
Summary
These changes aim to include type checks that were previously ignored
Testing
CI should be sufficient
Category
slack_bolt.Appand/or its core componentsslack_bolt.async_app.AsyncAppand/or its core componentsslack_bolt.adapter/docsRequirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
./scripts/install_all_and_run_tests.shafter making the changes.