-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
nixos/nix-daemon: Organize buildMachine options with a submodule #83104
Conversation
123393a
to
39d058d
Compare
502377a
to
5093da7
Compare
5093da7
to
9dc62cb
Compare
@msteen thanks for the review :) fixed all your points. I also changed |
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.
Seems good to me now!
9dc62cb
to
28a4c47
Compare
@Valodim thanks for picking this up for me! And @infinisil, thanks for letting me know. |
Can you add a 20.09 change log breaking change notice? |
I updated this to incorporate all feedback (and rebased on master). I think we actually ended up compatible with (sane usage of) the previous definitions, at least the config implementation looks identical modulo handling of defaults to me, and the example from the previous documentation produces identical results. There is a slight potential for errors in configs that used sloppy types before, e.g. a string value for
|
This PR picks up #47636. It changes the
nix.buildMachine
option from an attribute set to a submodule, with documentations and examples per option.A couple of thoughts:
types.secretPath
#78640system
andsystems
at the same time, because I couldn't figure out how to do assertions per module nicely :) I guess it's an artifact anyways that there are even two options for this, I'd rather deprecatesystems
, and typesystem
as str or listOf str, expecting the former to cover basically all cases.Ttested this locally, works as expected :)