Hello,
thanks to both ludo and ng0 looking at my patch.
24.10.2017 07:04 Ludovic Courtès:
Toggle quote (13 lines)
>> From 74618e5a39198077327f14362d8d98538f4d39ab Mon Sep 17 00:00:00 2001
>> From: nee <nee.git@cock.li>
>> Date: Sat, 14 Oct 2017 11:27:50 +0200
>> Subject: [PATCH] services: Add murmur.
>>
>> * gnu/services/telephony.scm: New file.
>> * gnu/local.mk: Add it.
>> * doc/guix.texi: Document it.
>
> You can write:
>
> * doc/guix.texi (Telephony Services): New node.
>
Okay, I changed this line in the commit message.
Toggle quote (13 lines)
>> +@deftp {Data Type} murmur-configuration
>> +The service type for the murmur server. An example configuration can look like this:
>> +@example
>> +(service murmur-service-type
>> + (murmur-configuration
>> + (welcome-text "Welcome to this mumble server running on GuixSD!")
>> + (cert-required #t) ; disallow text password logins
>> + (ssl-cert "/etc/letsencrypt/live/mumble.example.com/fullchain.pem")
>> + (ssl-key "/etc/letsencrypt/live/mumble.example.com/privkey.pem")))
>> +@end example
>
> Please don’t use tabs.
>
Toggle quote (12 lines)
>> +After reconfiguring your system, you have to manually set the
>> +SuperUser password with the command that is printed during the activation phase.
>
> That sounds quite unusual. Perhaps you need @code{SuperUser}, if you
> literally mean the “SuperUser” account in Mumble?
>
>> +Then you can use the @code{mumble} client to
>> +login as new user, register, and logout.
>> +For the next step login with the name "SuperUser" and the SuperUser password
>
> Same here.
>
I reworded that part a little. It's about the mumble "SuperUser" who can
create channels and do moderator stuff like muting, banning, and
promoting users.
Toggle quote (22 lines)
>> +(define-record-type* <murmur-configuration> murmur-configuration
>> + make-murmur-configuration
>> + murmur-configuration?
>> + (package murmur-configuration-package ;<package>
>> + (default mumble))
>> + (user murmur-configuration-user
>> + (default "murmur"))
>> + (group murmur-configuration-group
>> + (default "murmur"))
>> + (port murmur-configuration-port
>> + (default 64738))
>
> [...]
>
>> + (allow-html murmur-configuration-allow-html
>> + (default #f))
>> + (allow-ping murmur-configuration-allow-ping
>> + (default #f))
>
> Add a question mark since these are Boolean options. So ‘allow-html?’
> and ‘allow-ping?’.
>
Okay, I'm just slightly confused whether the question mark is only used
for predicate procedures or everything that related to booleans.
I think there was discussion on the guile list about this, I'll read up
on it later.
Toggle quote (22 lines)
>> +(define (default-murmur-config
>> + package user group port welcome-text server-password
>> + max-users max-user-bandwidth database-file log-file pid-file
>> + autoban-attempts autoban-timeframe autoban-time
>> + opus-threshold channel-nesting-limit channelname-regex username-regex
>> + text-message-length image-message-length cert-required
>> + remember-channel allow-html allow-ping bonjour send-version log-days
>> + obfuscate-ips ssl-cert ssl-key ssl-dh-params ssl-ciphers
>> + public-registration)
>
> This many positional parameters is not reasonable. :-) Just pass a
> <murmur-configuration> directly, and use the accessor procedures.
>
>> +(define murmur-activation
>> …
>
> Likewise: use the accessor procedures instead of this.
>
>> +(define murmur-shepherd-service
>> …
> Use the accessors instead.
>
Right, that grew way too big. I removed most of the match blocks.
I like having the short names when it comes to stitching together the
actual config though, so I kept that one.
If that's still a no-go I'll make another update with accessors.
If the main problem here is the positional binding, is there a function
to match record fields by name that I could use instead?
It doesn't seem like it would be too complicated to write a macro for
this with the record-accessor procedure from srfi-9.
Toggle quote (33 lines)
>> +(define murmur-accounts
>> + (match-lambda
>> + (($ <murmur-configuration> _ user group)
>> + (filter identity
>> + (list
>> + (and (equal? group "murmur")
>> + (user-group
>> + (name "murmur")
>> + (system? #t)))
>> + (and (equal? user "murmur")
>> + (user-account
>> + (name "murmur")
>> + (group group)
>> + (system? #t)
>> + (comment "Murmur Daemon")
>> + (home-directory "/var/empty")
>> + (shell (file-append shadow "/sbin/nologin")))))))))
>
>
> Why not just
>
> (match-lambda
> (($ <murmur-configuration> _ user group)
> (list (user-group (name group) (system? #t))
> (user-account
> (name user)
> (group group)
> (system? #t)
> …
> ))))
>
> ?
>
Okay I changed it. I had copied this from the fcgiwrap service.
Toggle quote (1 lines)
> Could you send an updated patch?
Here it is :-)
I also noticed a missing equal sign after rememberchannel in the
defaultconfig and added that.
