-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix(client/linters/mypy): call mypy incorrectly #5834
Conversation
It's supposed to call `mypy` in the root package directory with the relative path to the python file being checked. This commit enforces this behavior.
I couldn't really think of any suitable test case for this. Any suggestion? Or, can I just mark test case check as checked? |
Apologies for never getting back to you on this issue. Somehow we missed this. |
@yxliang01 I'll add the tests for the PR. |
@yxliang01 Here are the tests (You can add them Or i can after I merge your PR) DonJayamanne@9225b6e |
@DonJayamanne Hmm wondering how this going? |
@karthiknadig /cc |
@yxliang01 I added the tests and news item. You have to sign the CLA before i can merge. |
@DonJayamanne @karthiknadig CLA signed :) |
It's supposed to call
mypy
in the root package directory with the relative path to the python file being checked. This commit enforces this behavior.Has telemetry for enhancements.Test plan is updated as appropriatepackage-lock.json
has been regenerated by runningnpm install
(if dependencies have changed)The wiki is updated with any design decisions/details.Before this PR is merged, you may use https://gist.github.com/yxliang01/b975fffa0b145d75ed1dba5b3a9ad960 as the
mypy
wrapper to achieve the same effect.Thanks @harpaj for the idea.
Related to #5326