feat(core): add typing of core aspect keys in strict mode#449
feat(core): add typing of core aspect keys in strict mode#449DylanRJohnston wants to merge 5 commits into
Conversation
| den.schema.aspect.options.os = lib.mkOption { | ||
| type = lib.types.deferredModule; | ||
| default = { }; | ||
| }; |
There was a problem hiding this comment.
Not sure if it's possible, but it would be cool to only include the schema if the provider is being used. I think the only way to do it would be to turn it into a flakeModule?
There was a problem hiding this comment.
I don't think it is possible right now, aspects/providers cannot influence higher levels schemas, I'd like to know if someone finds a way to have that. could enable things like an aspect setting a host entity attribute if possible.
Do we need to make this options.os presence optional here? If so I guess you are right about flakeModules.
| defaultClasses.options = { | ||
| nixos = moduleOption; | ||
| darwin = moduleOption; | ||
| }; |
There was a problem hiding this comment.
Didn't know where to stick these base aspect class definitions, I don't think they should stay here.
There was a problem hiding this comment.
We can move those to /modules, those classes are very common, and we will likely always have those definitions loaded, so just create a file for them at modules.
| test-aspect-wsl = denTest ( | ||
| { inputs, igloo, ... }: | ||
| { | ||
| imports = [ | ||
| inputs.den.flakeModules.strict | ||
| inputs.den.flakeOutputs.all | ||
| ]; | ||
|
|
||
| den.hosts.x86_64-linux.igloo = { | ||
| wsl.enable = true; | ||
| users.tux = { }; | ||
| }; | ||
|
|
||
| den.aspects.igloo.wsl.defaultUser = "igloo"; | ||
|
|
||
| expr = | ||
| if inputs ? nixos-wsl then | ||
| igloo.wsl.defaultUser | ||
| else | ||
| lib.warn "nixos-wsl not found in inputs, skipping test `strict-mode.flakeModule.test-aspect-wsl" "igloo"; | ||
| expected = "igloo"; | ||
| } | ||
| ); |
There was a problem hiding this comment.
nixos-wsl isn't in our imports and nix-unit doesn't have a concept of "skipping a test for a reason", so this is the nearest thing I could come up with.
There was a problem hiding this comment.
Since nixos-wsl is not at inputs, we set host.wsl.module to a deferredModule that defines only wsl.enable and wsl.defaultUser, see files features/*wsl* for tests.
DylanRJohnston
changed the title
| throw '' | ||
| STRICT MODE | ||
| explanation = | ||
| if lib.lists.hasPrefix [ "flake" ] path then |
There was a problem hiding this comment.
I think using these ifs is the right way to do this, but I dont quite like large if branches, I'd prefer to have messages assigned as let bindings and have a short if chain at the end referencing those messages, that we can have visibility of what the if condition is doing.
| inherit (den.lib.ctxTypes nsCtxApply) ctxTreeType; | ||
| in | ||
| { | ||
| imports = [ (den.schema.namespace or { }) ]; |
|
Hey @DylanRJohnston we have finally merged The Big fx-pipeline branch #475. Do you want me to rebase and solve conflicts so we can keep progress on this ? |
|
Hello @DylanRJohnston I could use your review on: Will this satisfy your needs if we swap the current entity system for it? Also any review feedback you have on the design or implementation is 100% welcome. |
|
@DylanRJohnston no worries. If you have time to look at den-schema, it's a ground-up clean-room re-implementation of den's schema library that we're going to be subbing in and I believe it should cover your use-case requirements. Soliciting review to confirm that. :) |
3 participants
Follow up from #428, when I actually went to use this in my config I found that all the core aspect classes were missing explicit typing.