-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
revamp of adding type hints for SQLAlchemy models #9
base: main
Are you sure you want to change the base?
Conversation
CristhianMotoche
commented
Oct 3, 2024
- chore: Set up environment
- chore: Set up deps
- chore: Define model
- chore: Connect with App and get simple form
- chore: Install pyright
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @CristhianMotoche. This one looks great! I left some minor suggestions. Please take them a look. :)
reveal_type(get_all) | ||
|
||
reveal_type(get_unread) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to wrap these reveal_type
calls in an if TYPE_CHECKING:
conditional to avoid runtime errors. WDYT?
reveal_type(get_all) | |
reveal_type(get_unread) | |
if TYPE_CHECKING: | |
reveal_type(get_all) | |
reveal_type(get_unread) |
## Start | ||
|
||
``` | ||
devenv shell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to add a classical requirements.txt
flow for non-Nix users. WDYT?
|
||
|
||
if __name__ == "__main__": | ||
if len(sys.argv) > 1 and sys.argv[1] == 'db': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add instructions about this in the README?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this file in the repository?