-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Increase mypy_primer
comment length
#9598
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
Conversation
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
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.
Might be worth doing the same on mypy.
Also, over at Black we have a similar system called diff-shades, and there we get around the comment length limit by printing the full diff to the action's log instead and linking to it. It's a little less convenient, but we could consider it as an option for very long diffs if we think it's important to be able to see the full comment. I'd personally consider it overkill since a very long mypy-primer diff is usually enough indication that a change is a bad idea. |
Interesting, that sounds like overkill for typeshed, but might be pretty useful for mypy. It would have been very useful to easily see the full diffs for the |
Note that the action prints the full diff! This is purely for convenience with the comment since I hit the old limit recently with a legit PR. |
The full diffs are available for download if you click through. To Alex's suggestion for mypy: we now have per project truncation on mypy. Avasam, do you have a PR in mind where 30k chars more of comment would have been useful? |
I think probably #9597 |
~3.5k for that one in its current state (24 truncated lines). While working on it I easily had 80+ truncated lines of legit diff I had to go through. - <something about> Row
+ <something about> Row[Any] The idea is more that if we limit the length for technical reasons, may as well get closer to the limit, not halfway there (and it doesn't negatively affect readability). In other words, making the limit less arbitrary. Truncating in multiple comments is something that can be done if someone wants to tackle it, but this hard-limit would still be relevant, and is likely still gonna be hit from the homeassistant project alone once I tackle SQLAlchemy generics. |
To be exact, we only need to reserve 170 characters to truncate over a thousand lines. But I figured that
65300
was a nice round number with a good buffer.Sources:

and https://github.com/orgs/community/discussions/27190#discussioncomment-3254953