matrix-appservice-irc: init at 0.17.1 #89106
Conversation
| ''; | ||
|
|
||
| meta = with lib; { | ||
| description = "A Matrix <--> IRC bridge"; |
There was a problem hiding this comment.
| description = "A Matrix <--> IRC bridge"; | |
| description = "Node.js IRC bridge for Matrix"; |
Also please add homepage
|
|
||
| passwordKeyLength = mkOption { | ||
| type = ints.unsigned; | ||
| description = "Length of te key to encrypt IRC passwords"; |
There was a problem hiding this comment.
| description = "Length of te key to encrypt IRC passwords"; | |
| description = "Length of the key to encrypt IRC passwords"; |
Also what kind of encryption scheme is this? If RSA wouldn't 4096 be more appropriate these days?
|
|
||
| needBindingCap = mkOption { | ||
| type = bool; | ||
| description = "Whether the daemon needs to bind to ports less than 1024 (e.g. for the ident service)"; |
There was a problem hiding this comment.
Either
| description = "Whether the daemon needs to bind to ports less than 1024 (e.g. for the ident service)"; | |
| description = "Whether the daemon needs to bind to ports below 1024 (e.g. for the ident service)"; |
or
| description = "Whether the daemon needs to bind to ports less than 1024 (e.g. for the ident service)"; | |
| description = "Whether the daemon needs to bind to privileged ports (e.g. for the ident service)"; |
| wantedBy = [ "multi-user.target" ]; | ||
|
|
||
| preStart = '' | ||
| mask="$(umask)" |
There was a problem hiding this comment.
What is the reason for this assignment if you're not using it below?
| StateDirectory = "matrix-appservice-irc"; | ||
| StateDirectoryMode = "755"; | ||
|
|
||
| User = "appservice-matrix-irc"; |
There was a problem hiding this comment.
This seems to be reversed?
| User = "appservice-matrix-irc"; | |
| User = "matrix-appservice-irc"; |
Same for group.
| @@ -0,0 +1,27 @@ | |||
| # TODO: This has a bunch of duplication with matrix-appservice-slack | |||
There was a problem hiding this comment.
What exactly is problematic about this?
|
The module is also missing in |
|
One missing optional dependency that sound quite important:
[...]
|
| needBindingCap = mkOption { | ||
| type = bool; | ||
| description = "Whether the daemon needs to bind to ports less than 1024 (e.g. for the ident service)"; | ||
| default = false; |
There was a problem hiding this comment.
Shouldn't the default depend on the selected port? default = cfg.port < 1024
There was a problem hiding this comment.
No, because the option is unrelated to cfg.port. It's required when matrix-appservice-irc is supposed to run an identd server on 113/tcp. I wonder if we can get rid of this option however by parsing the configuration.
https://github.com/matrix-org/matrix-appservice-irc/blob/master/config.sample.yaml#L367-L384
|
Superseded by #95854. |
Motivation for this change
Another attempt at #62864.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)