-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
pid-fan-controller: init at 0.1.1 #336849
base: master
Are you sure you want to change the base?
Conversation
3af0028
to
875018c
Compare
875018c
to
a7e9668
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/4559 |
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.
style suggestions only, didn't test this
a7e9668
to
4137528
Compare
heat_srcs = lib.mkOption { | ||
type = lib.types.listOf (lib.types.submodule heatSource); | ||
description = "list of heat sources"; | ||
example = '' |
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.
The example is likely a big to big to properly render on search.nixos.org.
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.
I just halved the length of the example. Is there some example of the limits search.nixos.org is able to display?
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.
wdym? Yeah, it's a bit big, but nixos-search
and the manual should be fine with it.
4137528
to
2f41247
Compare
73e7806
to
3a6a475
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.
Overall this already looks really good to me.
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.
All of these are grammar, cosmetic or other nitpicks. None of them are blockers, and you should feel free to customise as needed.
3a6a475
to
15e0150
Compare
15e0150
to
c07f3a2
Compare
c07f3a2
to
fc855c7
Compare
fc855c7
to
dbcd6f0
Compare
dbcd6f0
to
210aa64
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.
Looks good to me! Thank you for addressing all of the reviews.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1973 |
Description of changes
pid-fan-controller is a (as the name implies) a fan controller using a PID control loop for Linux.
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.