[bug#37222] [PATCH] gnu: services: Add dkimproxy-out.

["Ludovic Courtès" <ludo@gnu.org>]

Hi!

Julien Lepiller <julien@lepiller.eu> skribis:

>>From 114067a7134ceb49dc5bbcef820edc49d62c8d0f Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Thu, 29 Aug 2019 21:48:25 +0200
> Subject: [PATCH] gnu: services: Add dkimproxy-out.
                   ^
No need for “gnu:”, which is only for (gnu packages …).  (Confusingly…)
>
> * gnu/services/mail.scm (dkimproxy-out-service-type): New variable.
> * doc/guix.texi (Mail Services): Document it.
> +@subsubheading Dkimproxy Outbound Service
> +@cindex Dkimproxy Outbound Service
> +

Could you add an introductory paragraph, for instance mentioning what
DKIM is about, linking to the Wikipedia page or something?

> +@deffn {Scheme Variable} dkimproxy-out-service-type
> +This is the type of the @uref{http://dkimproxy.sourceforge.net/, dkimproxy
> +outbound daemon}, whose value should be a @code{dkimproxy-out-configuration}
> +object as in this example:
> +
> +@example
> +(service dkimproxy-out-service-type
> +         (dkimproxy-out-configuration
> +	   (listen "127.0.0.1:10027")
   ^
No tabs please.  :-)

> +            %default-imap4d-config-file
> +	    
> +	    dkimproxy-out-service-type

Ditto (several occurrences in this file.)

> +(define (generate-map-file config filename)
> +  (apply plain-file filename
> +         (map (lambda (config)
> +                (match config
> +                  ((selector (config ...))
> +                   (string-append
> +                     selector " "
> +                     (string-join
> +                       (map generate-dkimproxy-out-signature-configuration config)
> +                       "\n")))
> +                  ((selector config)
> +                   (string-append
> +                     selector " "
> +                     (generate-dkimproxy-out-signature-configuration config)))))
> +              config)))

This is incorrect since ‘plain-file’ takes exactly two arguments.
Should it be something like:

  (plain-file file-name (string-join (map … config)))

?

> +                               (domains
> +                                 (apply append
> +                                   (map

Use ‘append-map’ instead.

> +(define dkimproxy-out-service-type
> +  (service-type
> +    (name 'dkimproxy-out)
> +    (extensions
> +      (list (service-extension account-service-type
> +                               (const %dkimproxy-accounts))
> +            (service-extension shepherd-root-service-type
> +                               dkimproxy-out-shepherd-service)))))

Please add a ‘description’ field.

It would be nice to have a system test too, which I guess could at least
ensure that the generated config is valid and that the daemon happily
starts?

Thanks,
Ludo’.