-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Sync mypy_primer_comment
with typeshed
#12125
Conversation
It looks like it still runs the old version: https://github.com/python/mypy/runs/5064684428?check_suite_focus=true |
Yes, actions run based on what's on master, not on the PR. I think this difference was intentional, perhaps because some mypy PRs (e.g., mypyc changes) don't affect the type checker and aren't expected to make a difference in mypy-primer. Then again, that's also true for some typeshed changes. |
I guess if that's an issue we can add |
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.
Yeah, the version of the action in the PR doesn't run to avoid security issues, since the action has write permissions on PRs.
I seem to remember some doubt about this being noisy, but I couldn't find where that was discussed and people on typeshed don't seem to mind, so let's go for it. Note that the real mypy-primer workflow already ignores mypyc, so we don't need to add files
here.
The one change I would like is to minimise differences in the comment workflow between this and typeshed. E.g., I think typeshed acquired some nice change that uses a glob instead of manually listing the shards / the comment hider workflow runs in a different order due to not hiding on mypy. Let's update this PR to make the mypy version match typeshed as much as possible
I will address this in a separate PR, because I will need to investigate the changes there. They seem to be rather big 🤔 |
I am going to merge it and test with my upcoming |
Thanks, sounds good, I'm familiar with the comment hiding stuff, so can sync it myself :-) |
@hauntsaninja I am working on right now 😊 |
This PR adds a useful feature: now empty
primer
output will still add a comment: "According to mypy_primer, this change has no effect on the checked open source code."Why is it useful?
Because in some cases it is not clear: is
mypy_primer
happy? Or is it broken?For example, this PR #12118 does not have any comment.
I guess it would also benefit PR authors who can be sure that what they did is correct.
I will wait for
mypy_primer
to comment on this PR 🙂