-
-
Notifications
You must be signed in to change notification settings - Fork 269
stylix: add module maintainers to corresponding testbed derivations #1485
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
base: master
Are you sure you want to change the base?
stylix: add module maintainers to corresponding testbed derivations #1485
Conversation
a6381e1
to
60dbddb
Compare
On second thought, I would prefer using diff --git a/stylix/testbed/default.nix b/stylix/testbed/default.nix
index cb57c27..b4a0189 100644
--- a/stylix/testbed/default.nix
+++ b/stylix/testbed/default.nix
@@ -97,16 +97,8 @@ let
};
in
lib.optionalAttrs (isEnabled testbed.path) {
- # For performance reasons, the more convenient and generic
- # lib.recursiveUpdate function is not used as follows:
- #
- # lib.recursiveUpdate script {
- # meta.maintainers = meta.${testbed.module}.maintainers;
- # }
- ${name} = script // {
- meta = script.meta // {
- inherit (meta.${testbed.module}) maintainers;
- };
+ ${name} = lib.recursiveUpdate script {
+ meta.maintainers = meta.${testbed.module}.maintainers;
};
}; |
60dbddb
to
f4a22f2
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.
doesn't really seem necessary imo, but I also don't see any downside
Is it not possible to pass |
Seems like a nice addition.
This would imply being a maintainer of the ${name} = script; is ever extended to be something like ${name} = pkgs.buildEnv {
inherit name;
paths = [script /* Filesystem of the testbed */];
}; it would silently work not as intended. Actually, I stumbled accross the maintainer addition when trying to add the testbed's filesystem under |
f4a22f2
to
ad5c46c
Compare
Regarding the performance comment about
lib.recursiveUpdate
, should we still uselib.recursiveUpdate
since thescript
attribute set is fairly small? For reference, here is the relevantlib.recursiveUpdate
implementation:The evaluation of all testbed maintainer values can be verified with the following
nix repl
command:Things done
Notify maintainers