Skip to content
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

Merge original typing from types-redis, adapt it for valkey. #134

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Rafiot
Copy link

@Rafiot Rafiot commented Dec 3, 2024

Replacement PR for #93, this time with signoff (sorry for the noise, I had a fight with git and it seems to be the easiest way out).

Still not ready for merging.

@Rafiot
Copy link
Author

Rafiot commented Dec 3, 2024

@amirreza8002 Let's discuss about the state of the PR here.

So my approach was to use the test suite as a baseline to have working type hints, so I gradually enable mypy on the files in the test suite: https://github.com/Rafiot/valkey-py/blob/types-signed-off-take2/.mypy.ini

And adapt either the test suite, or the typing, depending on the issues reported by mypy.

One day, we should try to get rid of the .pyi files, but it's gonna be tricky due to the way the sync and async modules are implemented.

@amirreza8002
Copy link
Contributor

hi
thanks for the info

and what parts do you need help with?

or what parts you aren't gonna do for now? so we don't end up working on the same thing

@Rafiot
Copy link
Author

Rafiot commented Dec 3, 2024

I'm working on something else right now, so I don't have anything going that is not in this PR. Feel free to work on anything you want.

The next steps are to run mypy on more of the test files, so feel free to pick anything you want. If you have something ongoing and you don't push it same day, please add a comment here so we don't duplicate the work.

Thanks!

The important thing to keep in mind is that you always signoff the commits, it's required to merge it, and it is a mess to fix later on (as I realized today).

@Rafiot Rafiot force-pushed the types-signed-off-take2 branch from f271a43 to e9272c7 Compare December 16, 2024 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants