Re: [PATCH] improve nginx-service

[ludo@gnu.org (Ludovic Courtès)]

Hi Julien,

This looks like a great improvement to me!  Sounds nicer than fiddling
with config files.

I suppose we could make ‘nginx-service-type’ extensible (info "(guix)
Service Types and Services") so that people can write services that
define new vhosts?

Julien Lepiller <julien@lepiller.eu> skribis:

> From 613d5db739d4010be2037fd2fcfc70baca4625aa Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Mon, 26 Sep 2016 23:55:58 +0200
> Subject: [PATCH] services: improve nginx-service configuration
>
> * gnu/services/web.scm (<nginx-vhost-configuration>): New record type.
> * doc/guix.texi (Web Services): Document 'nginx-vhost-configuration'.

Please mention the other changes in web.scm: new procedures, changes in
existing procedures, etc.

> +As an alternative to using a @var{config-file}, @var{vhost-list} can be
> +used to specify the list of vhosts required on the host.  For this to work,

“the list of @dfn{virtual hosts} or @dfn{vhosts}”

> +@deftp {Data Type} nginx-vhost-configuration
> +Data type representing the configuration of an nginx virtual host.

@dfn{virtual host}

> +This type has the following parameters:
> +@table @asis
> +@item @code{http-port} (default: 80)

@code{80}; likewise for all the other default values.

> +Nginx will listen for http connection on this port. Set it at #f if nginx should
> +not listen for http (non secure) connection for this vhost.

s/http/HTTP/ and s/#f/@code{#f}/
Please leave two spaces after an end-of-sentence period.

> +(define (list-names names)
> + (match names
> +  ('() "")
> +  ((head tail ...) (string-append (if (eq? head 'default) "_" head)
> +                                  " "
> +                                  (list-names tail)))))

Maybe call it ‘config-value-strings’?  Please add a docstring.  I think
it’d be more readable like this:

  (define (config-value-strings names)
    "Return a string denoting the nginx config representation of NAMES,
   a list of foobars…"
    (string-concatenate
     (map (match-lambda
            ('default "_")
            ((? string? str) str))
          names)))

Could you send an updated patch?

Unless David, who initially wrote this service has objections, this
patch looks good to me with the changes as suggested above.

Thanks!

Ludo’.