-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add mypy check to github actions #177
Add mypy check to github actions #177
Conversation
Pull Request Test Coverage Report for Build 942
💛 - Coveralls |
Hi @avara1986 I have added the mypy checks into github actions, still working on reducing the errors & adding more type hints. However I have some questions regarding the pipfile and the lock file. Not too familiar with it as I normally use pip & virtualenv in my own projects. But I do understand the concept of a lock file from other package managers like yarn. When I generated the lockfile I see that not only did i add the mypy dependency, I also upgraded the minor versions of other dependencies (which should be non-breaking). But what I would like to ask is what the My python env is 3.7.5 |
Hi @ooigavin, It's normal that if you run
The most of pyms dependencies are "greater or equeal to [version]" and each time you run pipenv The new I hope this help 😄 |
Goddit, thanks! one other thing I wanted to check, I am used to creating small commits for each change i make. However sometimes I think maintainers prefer PRs to contain a single commit? Not sure what is the convention for this project. |
IMHO, i like both. One big PR or a lot of small PRs 😃 BUT, if you are playing the Hacktoberfest I think a lot of small PRs with one or two lines updated could be considered as SPAM 👍 |
oh what i meant is in a single PR, is it alright if it contains many commits, or should it just contain one commit containing all the changes? Only intend to submit a single PR for this |
I read that i want sorry....:joy: No, you can do all commits that you want, It's ok 1 or 1000 :+1: |
Have added more type hints and reduced some of the mypy errors, left some whr I wasnt too sure about the typing alone. |
Great PR @ooigavin ! is ready to review and merge or do you want to add more commits? 👍 |
I've looked thru most of the codebase, and added the types whr I was confident to add them. I'm ready to merge, unless you need me to make any changes? |
Fixes # 166
Changes proposed in this PR: