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
nixos/logrotate: switch paths
option type from listOf to attrsOf
#95809
Conversation
381e3c8
to
62c1b13
Compare
@infinisil we had a small conversation about this module and I always appreciate/enjoy your reviews... would you mind taking a look at this? |
Thanks for your review @infinisil. I have made a few fixes/additions as per your comments. I have also addressed some of the comments which I think might not be the best direction for this module. Please let me know your thoughts. |
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 first commit looks good now, but I can't really comment much on the second one as I don't know a lot about httpd. If you think the second one won't cause any problems then feel free to merge it :)
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. Thanks!
paths
option type from listOf to attrsOf (submodule ...)paths
option type from listOf to attrsOf
This seems to have broken the nixos manual: https://hydra.nixos.org/build/125616472/nixlog/5 |
Motivation for this change
#87702
It was a really bad idea for me to suggest @jslight90 to use a
listOf
instead ofattrsOf
for thepaths
option. I upgraded a server tounstable
and tried this out...attrsOf
is a much better fit when given apriority
option to control sort order.With this PR NixOS modules can actually start providing log rotation for (the very few) services that actually need it using
logrotate
. The user remains in full control with the ability to even disable log rotation via theenable
option, like so:Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)