-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Change mypy session to log all errors. #2315
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2315 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 2454 2454
=========================================
Hits 2454 2454 Continue to review full report at Codecov.
|
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.
So exciting!! Looks like there's a lint failure too.
@@ -159,13 +124,9 @@ def mypy(session): | |||
# Ensure that mypy itself ran successfully | |||
assert process.returncode in (0, 1) | |||
|
|||
for line in process.stdout.split("\n"): |
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.
Can we change all this logic into a session.run(mypy --strict src/)
?
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.
What kind of change do you mean?
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.
Instead of capturing the output of mypy and doing all this filtering we now don't expect any mypy errors so in theory can use session.run()
directly?
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.
Amazing to see a ✅ next to the lint job, great work @hramezani 👏👏👏
Closes #1897