Hi Florian "pelzflorian (Florian Pelz)" <pelzflor...@pelzflorian.de> skribis:
>>From 9ec69c888b978cb870a5873af8e327541fe4ef7a Mon Sep 17 00:00:00 2001 > From: Florian Pelz <pelzflor...@pelzflorian.de> > Date: Sun, 6 Oct 2019 20:45:34 +0200 > Subject: [PATCH 1/2] [wip] gnu: Add ngx_http_accept_language_module. > > * gnu/packages/web-xyz.scm: New file. > * gnu/local.mk (GNU_SYSTEM_MODULES): Add package. [...] > +++ b/gnu/packages/web-xyz.scm > @@ -0,0 +1,175 @@ > +;;; GNU Guix --- Functional package management for GNU > +;;;; TODO should I really add copyright lines for people I copied from?? > +;;; Copyright © 2014, 2015 Mark H Weaver <m...@netris.org> > +;;; Copyright © 2016 Tobias Geerinckx-Rice <m...@tobias.gr> > +;;; Copyright © 2017, 2018 Marius Bakke <mba...@fastmail.com> I don’t think you need to add these 3 lines here; the package definition is yours. > +(define-public nginx-mod-accept-language > + (let ((commit "2f69842f83dac77f7d98b41a2b31b13b87aeaba7") > + (revision "1")) Is there no upstream version? If that’s the case, that’s fine, but please add a comment explaining it. > + (package > + (name "nginx-mod-accept-language") Perhaps “nginx-accept-language-module”, to match the name of the upstream repo? > + (modules '((guix build utils) > + (ice-9 popen))) > + (snippet > + #~(begin > + ;; the nginx source code is part of the module’s source > + (format #t "decompressing nginx source code~%") > + (call-with-output-file "nginx.tar" > + (lambda (out) > + (let ((pipe (open-pipe* OPEN_READ > + #+(file-append gzip "/bin/gzip") > "-cd" > + #$(package-source nginx)))) > + (dump-port pipe out) > + (unless (= (status:exit-val (close-pipe pipe)) 0) > + (error "gzip decompress failed"))))) > + (invoke #+(file-append tar "/bin/tar") "xvf" "nginx.tar" > + "--strip-components=1") > + (delete-file "nginx.tar") I’d suggest doing it in a phase. > + (license (delete-duplicates > + (cons license:bsd-2 ;license of nginx-mod-accept-language > + (package-license nginx))))))) ;the module’s code is > linked To avoid circular dependencies in top-level references, I suggest copying the license of ‘nginx’ instead of writing (package-license nginx). > + nginx-configuration-load-modules > nginx-configuration-extra-content > nginx-configuration-file > > @@ -522,6 +524,7 @@ > (default #f)) > (server-names-hash-bucket-max-size > nginx-configuration-server-names-hash-bucket-max-size > (default #f)) > + (load-modules nginx-configuration-load-modules (default '())) What about “loaded-modules”, “loadable-modules”, or simply “modules”? “load-modules” sounds imperative whereas the rest of the config is declarative. Apart from that it LGTM. >>From ea5edd15586722b3557912e81171e69f7be339fa Mon Sep 17 00:00:00 2001 > From: Florian Pelz <pelzflor...@pelzflorian.de> > Date: Mon, 7 Oct 2019 07:58:30 +0200 > Subject: [PATCH] berlin: Redirect to localized website depending on > Accept-Language header. > > * hydra/nginx/berlin.scm (guix.gnu.org-locations): Redirect html URLs. > (%nginx-configuration): Load required nginx dynamic module. LGTM, but I guess we’ll commit it when we’re ready to switch to the new web site. Thanks! Ludo’.
