Skip to content
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

Make ConfigSetting generic #3215

Merged
merged 1 commit into from
Nov 22, 2024
Merged

Make ConfigSetting generic #3215

merged 1 commit into from
Nov 22, 2024

Conversation

septatrix
Copy link
Contributor

This prevents one from accidentally using incompatible parse/match/default_factory callbacks. A few type ignores are necessary due to python/mypy#3737 and in finalize_config because any inference is impossible there

@septatrix septatrix force-pushed the main branch 2 times, most recently from db14098 to f1882f2 Compare November 20, 2024 20:53
@septatrix septatrix marked this pull request as ready for review November 20, 2024 20:54
@septatrix septatrix force-pushed the main branch 3 times, most recently from 0a8e11c to 403500f Compare November 20, 2024 22:14
Copy link
Contributor

@behrmann behrmann left a comment

Choose a reason for hiding this comment

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

I really like this! I was holding off from adding generics, since this has changed so much from our lower bar of 3.9 to now, that I didn't think it was worth it, but looks like it already works somewhat sufficiently. Nice!

mkosi/config.py Show resolved Hide resolved
mkosi/config.py Outdated Show resolved Hide resolved
mkosi/config.py Outdated Show resolved Hide resolved
mkosi/config.py Outdated Show resolved Hide resolved
mkosi/config.py Show resolved Hide resolved
mkosi/config.py Outdated Show resolved Hide resolved
@behrmann
Copy link
Contributor

One thing I forgot: This should probably be checked on Python 3.9 to ensure this doesn't introduce anything by chance that fails on 3.9.

@septatrix
Copy link
Contributor Author

One thing I forgot: This should probably be checked on Python 3.9 to ensure this doesn't introduce anything by chance that fails on 3.9.

Mypy and pyright both assume Python 3.9 in CI

mkosi/config.py Outdated Show resolved Hide resolved
@septatrix septatrix force-pushed the main branch 2 times, most recently from bbd21a7 to 72eb7a9 Compare November 21, 2024 13:39
mkosi/config.py Outdated Show resolved Hide resolved
@behrmann
Copy link
Contributor

behrmann commented Nov 21, 2024

Mypy and pyright both assume Python 3.9 in CI

Yeah, but type checking and actually running without throwing an exception (e.g. just the help output), are two different things, unfortunately. We've had plenty of cases where successfully type-checked code crashed because of circular imports right at import time. :)

@DaanDeMeyer DaanDeMeyer merged commit a86d3dd into systemd:main Nov 22, 2024
33 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants