[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] improve nginx-service
From: |
Ludovic Courtès |
Subject: |
Re: [PATCH] improve nginx-service |
Date: |
Wed, 19 Oct 2016 23:04:14 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
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 <address@hidden> skribis:
> From 613d5db739d4010be2037fd2fcfc70baca4625aa Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <address@hidden>
> 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}”
> address@hidden {Data Type} nginx-vhost-configuration
> +Data type representing the configuration of an nginx virtual host.
@dfn{virtual host}
> +This type has the following parameters:
> address@hidden @asis
> address@hidden @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’.
- [PATCH] improve nginx-service, Julien Lepiller, 2016/10/16
- Re: [PATCH] improve nginx-service,
Ludovic Courtès <=
- Re: [PATCH] improve nginx-service, Julien Lepiller, 2016/10/20
- Re: [PATCH] improve nginx-service, Ludovic Courtès, 2016/10/24
- Re: [PATCH] improve nginx-service, Julien Lepiller, 2016/10/26
- Re: [PATCH] improve nginx-service, Ludovic Courtès, 2016/10/27
- Re: [PATCH] improve nginx-service, Julien Lepiller, 2016/10/27
- Re: [PATCH] improve nginx-service, Ludovic Courtès, 2016/10/30