Hi :)
I made an update to this patch before I saw your feedback. I fixed some
things. Some comments I will incorporate without reply. The mail below
replies only to those remaining parts.
On Thu 06 Oct 2016 22:25, l...@gnu.org (Ludovic Courtès) writes:
>> +(define %cups-activation
>> + ;; Activation gexp.
>> + #~(begin
>> + (use-modules (guix build utils))
>
> To be sure:
>
> (with-imported-modules '((guix build utils))
> #~(begin …))
OK :)
> Could you add a comment on why we need to create this X.509 certificate
> and what it’s used for?
Sure. (It's for HTTPS access to localhost:631.)
> Would it be useful to allow for some parameterization (key type and
> size, “-days” value(?), etc.)?
Not sure. Currently generation of this cert happens automagically and
without parameters. I would leave this to a follow-up, but yeah, this
procedure should eventually live elsewhere.
>
>> +(define* (cups-service #:key (config (cups-configuration)))
>> + "Return a service that runs @var{cups}, the Cups database server.
>> +
>> +The Cups daemon loads its runtime configuration from @var{config-file}
>> +and stores the database cluster in @var{data-directory}."
>> + (validate-configuration config
>> + (if (opaque-cups-configuration? config)
>> + opaque-cups-configuration-fields
>> + cups-configuration-fields))
>> + (service cups-service-type config))
>
> s/Cups/CUPS/
>
> Nowadays I prefer to advertise the ‘service’ form so that users clearly
> see what’s going on. However, there’s the extra validation step here.
>
> Would it work to rename the real record constructors to
> ‘%cups-configuration’ and ‘%opaque-cups-configuration’, and then:
>
> (define-syntax-rule (cups-configuration fields ...)
> (let ((config (%cups-configuration fields ...)))
> (validate-configuration config …)
> config))
>
> … in which case we can remove the ‘cups-service’ procedure and instead
> document:
>
> (service cups-service-type config)
>
> WDYT?
Sure :)
Tx for the review!
A
