[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] improve nginx-service
From: |
Julien Lepiller |
Subject: |
Re: [PATCH] improve nginx-service |
Date: |
Thu, 20 Oct 2016 14:37:44 +0200 |
On Wed, 19 Oct 2016 23:04:14 +0200
address@hidden (Ludovic Courtès) wrote:
> 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?
You mean something like udev-service-type where you could extend it
with a list of vhosts?
Except for that comment, I modified the patch following your advice.
>
> 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’.
0001-services-improve-nginx-service-configuration.patch
Description: Text Data
- [PATCH] improve nginx-service, Julien Lepiller, 2016/10/16
- Re: [PATCH] improve nginx-service, Ludovic Courtès, 2016/10/19
- Re: [PATCH] improve nginx-service,
Julien Lepiller <=
- 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