-
Notifications
You must be signed in to change notification settings - Fork 31
refactor: updating typing for xreadgroup / xread / xadd #130
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
refactor: updating typing for xreadgroup / xread / xadd #130
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #130 +/- ##
==========================================
- Coverage 76.20% 76.20% -0.01%
==========================================
Files 130 130
Lines 33909 33911 +2
==========================================
Hits 25842 25842
- Misses 8067 8069 +2 ☔ View full report in Codecov by Sentry. |
|
After running this a bit while developing interactions with valkey streams I think the types here seem to work and prevent incorrect types getting in. I've been testing with pyright, tho - not mypy. Not sure how to validate with mypy. |
|
Ah wait, perhaps I shouldn't have merged - that won't have the sign off 🤦 Let me know if you'd prefer a rebase. EDIT: Yeah I'll just rebase so I get the build to run in my local repo |
the typing for these functions are invariant and cannot be used with a typed dict as easily because of that Signed-off-by: James Ward <[email protected]>
|
The changes look good to me. @Rafiot could you take a look and tell if the changes look good to you as well? |
|
Yep, looks fine to me too. |
mkmkme
left a comment
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.
Thanks!
Pull Request check-list
Description of change
the typing for these functions are invariant and cannot be used with a typed dict as easily because of that
fixes #129