chore: improve type checking behavior - #1470
Conversation
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ 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. |
Sorry, something went wrong.
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 🚢
Sorry, something went wrong.
| 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!
Sorry, something went wrong.
| 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?
Sorry, something went wrong.
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
Sorry, something went wrong.
| 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!
Sorry, something went wrong.
Summary
These changes aim to include type checks that were previously ignored
Testing
CI should be sufficient
Category
Requirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.