Skip to content

Conversation

@tungol
Copy link
Contributor

@tungol tungol commented Nov 20, 2024

Another messy __all__. I tried to order the branches in a systematic way of increasing complexity: first version_info checks, then single platform and their versions, sorted alphabetically, then negative checks, then combinations, then or statements. No nesting the branches except where unavoidable (for complicated and/or conditions).

Side note, how are feeling about the precedent from #2286 to not use sys.platform checks on os.O_*? Some of them are certainly correct to be that way, but I feel like it's a little silly to not use the check when we can be confident about it.

This covers darwin. Linux and win32 will need cleanup still.
@tungol tungol marked this pull request as draft November 20, 2024 20:11
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@tungol tungol marked this pull request as ready for review November 20, 2024 22:07
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

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

@srittau srittau merged commit d84476c into python:main Nov 21, 2024
62 checks passed
@srittau
Copy link
Collaborator

srittau commented Nov 21, 2024

Side note, how are feeling about the precedent from #2286 to not use sys.platform checks on os.O_*? Some of them are certainly correct to be that way, but I feel like it's a little silly to not use the check when we can be confident about it.

Platform checks are preferable, in my opinion. Back in 2018, we didn't have primer stubtest to verify our assumptions. Of course, some flags might still be awkward and their existence depends on the build and/or runtime environment, or the phase of the moon, but we should do what we can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants