Re: [PATCH] gnu: Add CUPS service.

[Andy Wingo]

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, address@hidden (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