FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

socket: rework to match docs more closely, improve accuracy on linux by hauntsaninja · Pull Request #11219 · python/typeshed · GitHub

  1. python
  2. typeshed

Repository navigation

socket: rework to match docs more closely, improve accuracy on linux - #11219

Merged
hauntsaninja merged 7 commits into
python:mainfrom
hauntsaninja:socket-more
Jan 4, 2024
ConversationCommits (7)ChecksFiles changed
Merged

socket: rework to match docs more closely, improve accuracy on linux#11219
hauntsaninja merged 7 commits into
python:mainfrom
hauntsaninja:socket-more

Conversation

hauntsaninja commented Jan 2, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Match the documentation more closely

Fixes some of #8101 , in particular I no longer see any missing constants on the Linux machines I have access to

[socket] rework
e480f84 
Match the documentation more closely

Fixes some of python#8101
hauntsaninja changed the title [socket] rework socket: rework to match docs more closely, improve accuracy on linux Jan 2, 2024
hauntsaninja marked this pull request as ready for review January 2, 2024 10:09

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

github-actions Bot commented Jan 2, 2024

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

trio (https://github.com/python-trio/trio)
+ src/trio/_socket.py:376: error: Unused "type: ignore" comment  [unused-ignore]

hauntsaninja commented Jan 4, 2024

Copy link
Copy Markdown
Collaborator Author

(This PR might be a little tricky to review, but I'm fairly confident. We check socket with stubtest quite tightly on macOS and Windows, and I've manually confirmed we fix stuff without regressions on Linux. Primer is also good)

srittau approved these changes Jan 4, 2024
View reviewed changes

srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, when skimming over it and going by your comment and the primer output. Please merge in a day or two, unless someone else wants to do a deep review.

hauntsaninja mentioned this pull request Jan 4, 2024
Merged
AlexWaygood approved these changes Jan 4, 2024
View reviewed changes

AlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Haven't done a proper review here (and I can't without access to a Linux machine :), but this all looks highly plausible, and I trust your judgement + stubtest!

How many stubtest errors would there be in CI if we removed the blanket allowlist entries for linux now, do you know? Is it low enough that we could remove the blanket entries and just enumerate all the errors? (Doesn't need to be done in this PR, I'm just curious!)

hauntsaninja merged commit 2cafcc9 into python:main Jan 4, 2024
hauntsaninja deleted the socket-more branch January 4, 2024 22:36

hauntsaninja commented Jan 4, 2024

Copy link
Copy Markdown
Collaborator Author

Yes, just enumerating all the errors would be great. There's still quite a few errors... this PR only eliminates ones that are certainly errors, where we're missing things in the stubs.

The cases where we're missing things at runtime are trickier, because I don't know whether that's just the Linux machines I have access to or if it's real. So I might try to whittle down what I can, and when I give up I'll just allowlist how many ever are left.

bytemarx mentioned this pull request May 8, 2024
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

srittau srittau approved these changes
AlexWaygood AlexWaygood approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL