8000 Return 0 if there are only notes and no errors by ilevkivskyi · Pull Request #13879 · python/mypy · GitHub
[go: up one dir, main page]
Skip to content

Return 0 if there are only notes and no errors #13879

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 14, 2022
Merged

Return 0 if there are only notes and no errors #13879

merged 2 commits into from
Oct 14, 2022

Conversation

ilevkivskyi
Copy link
Member
@ilevkivskyi ilevkivskyi commented Oct 12, 2022 •
edited
Loading

Fixes #10013

See #13851 (comment) for motivation, also this sounds generally reasonable.

@ilevkivskyi ilevkivskyi mentioned this pull request Oct 12, 2022
Merged
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

hauntsaninja
hauntsaninja reviewed Oct 12, 2022
View reviewed changes
Copy link
Collaborator
@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks like this also closes #10013

mypy/main.py
@@ -110,10 +110,10 @@ def main(
print_memory_profile()

code = 0
if messages:
n_errors, n_notes, n_files = util.count_stats(messages)
if messages and n_notes < len(messages):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: from the impl, it looks like it's not super guaranteed that n_errors + n_notes == len(messages), maybe clearer to phrase as if messages and n_errors?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this is intentional. This case is complementary to the check for format_success() below. So that if something weird happens we both:

  • Do not show Success message
  • Return non 0 code

@JukkaL
Copy link
Collaborator
JukkaL commented Oct 13, 2022

What about notes from reveal_type()? I think that these are dangerous, since they generally will fail at runtime. Should they still result in a failed run?

@hauntsaninja
Copy link
Collaborator
hauntsaninja commented Oct 13, 2022 •
edited
Loading

We have at least one feature request for reveal_type to result in exit code 0: #10013
We also want exit code 0 when typing.reveal_type is used. So I'd say this is fine — and we can always make builtins.reveal_type emit an error.

@ilevkivskyi
Copy link
Member Author

Yes, I agree with @hauntsaninja, it is what people probably expecting (also I would propose to deprecate reveal_type() in favor of typing.reveal_type()).

@ilevkivskyi
Copy link
Member Author

I will just go ahead and merge this. It will be easy to undo if someone will complain.

@ilevkivskyi ilevkivskyi merged commit 201d116 into python:master Oct 14, 2022
@ilevkivskyi ilevkivskyi deleted the notes-ok branch October 14, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hauntsaninja hauntsaninja hauntsaninja left review comments

@JukkaL JukkaL Awaiting requested review from JukkaL

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using reveal_type with no errors should return exit code 0
3 participants
@ilevkivskyi @JukkaL @hauntsaninja
0