Subject: bug#26302: [website] translations

From: Ludovic Courtès <[email-masked]>
Date: Fri, 01 Nov 2019 15:54:42 +0100

Hi Florian

"pelzflorian (Florian Pelz)" <[email-masked]> skribis:

>>>From 9ec69c888b978cb870a5873af8e327541fe4ef7a Mon Sep 17 00:00:00 2001
> From: Florian Pelz <[email-masked]>
> 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 <[email-masked]>
> +;;; Copyright © 2016 Tobias Geerinckx-Rice <[email-masked]>
> +;;; Copyright © 2017, 2018 Marius Bakke <[email-masked]>

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 <[email-masked]>
> 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’.