-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
lib/types: init defaultComposedFunctor for types.{attrsWith,listOf} #366015
Conversation
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.
Discussed this together in a meeting with @hsjobeki:
- This should only be an internal refactoring, no new function should be exposed
- The function can be specific to an
elemType
payload, e.g.elemTypeFunctor
, which can then also implement the merge forelemType
specifically - The deprecation message makes sense to have for types that currently use
wrapped
In the future we could also imagine an interface like this for creating types with appropriate functors more easily:
submoduleWith = createAttrType {
# Doesn't need to stay in sync with `submoduleWith` anymore
# because the name is decoupled from the `lib.types` identifier
name = "submodule";
payloadSpec = {
modules = {
# No default, required argument
binOp = a: b: a ++ b;
};
specialArgs = {
default = {};
binOp = unionOfDisjoint;
}
# ...
};
# Gets a functor/typeMerge based on the above spec
create = { modules, specialArgs }: mkOptionType {
description = "...";
merge = "...";
# ...
};
};
204bd60
to
3f80731
Compare
7679fed
to
66be74f
Compare
66be74f
to
5978559
Compare
5978559
to
557e4fa
Compare
557e4fa
to
327883c
Compare
327883c
to
5782ef8
Compare
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.
Reviewed and updated together in a meeting with @hsjobeki, looking good now!
Overall i think all types.functor should deprecate
functor.wrapped
This PR introduces a new
defaultFunctor
for composed types that makes the migration easy.Additional thoughts:
types should be constructible via
type payload
maybe we want to spread the{type}With payload
pattern ?This design thought also went into the default functor:
Compare:
attrsWith
listWith
(doesnt exist)submoduleWith
deferredModuleWith
stringWith
(doesnt exist)This only internally changes the following for now.
listOf
requires overriding thetype
because there is nolistWith
yet.attrsWith
doesn't require any additional attribute overrides. (Only the custom binOp for merging)Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.