-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
clight: init #64309
clight: init #64309
Conversation
87e916d
to
bf61cfe
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.
Yeah something like that, nice
d82f166
to
47b7ed1
Compare
751121b
to
41c8e01
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.
Still some things to complain about, I hope you don't mind. Overall this PR looks very good, nice work! Feel free to ping me when you did the changes, GitHub doesn't send notifications just from pushing new commits unfortunately
Once everything is addressed, I'll do a final test to double check various combinations of options work and merge it afterwards assuming no problems
59f30d7
to
ae91a33
Compare
And while testing this I found these changes necessary too: diff --git a/nixos/modules/rename.nix b/nixos/modules/rename.nix
index faa2ea82181..6228c95ae91 100644
--- a/nixos/modules/rename.nix
+++ b/nixos/modules/rename.nix
@@ -258,8 +258,18 @@ with lib;
(mkRenamedOptionModule [ "networking" "resolvconfOptions" ] [ "networking" "resolvconf" "extraOptions" ])
# Redshift
- (mkAliasOptionModule [ "services" "redshift" "latitude" ] [ "location" "latitude" ])
- (mkAliasOptionModule [ "services" "redshift" "longitude" ] [ "location" "longitude" ])
+ (mkChangedOptionModule [ "services" "redshift" "latitude" ] [ "location" "latitude" ]
+ (config:
+ let value = getAttrFromPath [ "services" "redshift" "latitude" ] config;
+ in if value == null then
+ throw "services.redshift.latitude is set to null, you can remove this"
+ else builtins.fromJSON value))
+ (mkChangedOptionModule [ "services" "redshift" "longitude" ] [ "location" "longitude" ]
+ (config:
+ let value = getAttrFromPath [ "services" "redshift" "longitude" ] config;
+ in if value == null then
+ throw "services.redshift.longitude is set to null, you can remove this"
+ else builtins.fromJSON value))
] ++ (flip map [ "blackboxExporter" "collectdExporter" "fritzboxExporter"
"jsonExporter" "minioExporter" "nginxExporter" "nodeExporter"
diff --git a/nixos/modules/services/x11/redshift.nix b/nixos/modules/services/x11/redshift.nix
index e46c70e5f3b..55f8f75021b 100644
--- a/nixos/modules/services/x11/redshift.nix
+++ b/nixos/modules/services/x11/redshift.nix
@@ -89,7 +89,7 @@ in {
systemd.user.services.redshift =
let
providerString = if lcfg.provider == "manual"
- then "${lcfg.latitude}:${lcfg.longitude}"
+ then "${toString lcfg.latitude}:${toString lcfg.longitude}"
else lcfg.provider;
in
{ The first one to convert from the strings services.redshift.* to the float options location.*, and the second one to have the floats be turned to strings in redshift |
Also includes geolocation information abstracted from redshift.nix
Applied patch. |
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.
Thanks for the patience :)
is there any documentation on how this can be used? I was using provider on redshift service and have no idea how to make it compatible with this change. |
@turboMaCk You should only be getting a warning, no errors, so this is backwards compatible. Is that the case? |
I got error
Anyway I think I've got it working by using |
This was an oversight in NixOS#64309 resulting it backwards incompatibilities
@turboMaCk Ah thanks for the report, this was a mistake in this PR, #68852 fixes it |
I scared myself so many times before remembering the webcam lighting up was due to the program.
Motivation for this change
Auto adjusting brightness is one fine feature (or at least in my opinion).
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)