GNU bug report logs - #33185
[PATCH 0/7] Add patchwork package and service.

Previous Next

Full log

Message #107 received at 33185 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 33185 <at> debbugs.gnu.org
Subject: Re: [bug#33185] [PATCH v2 1/2] gnu: Add patchwork.
Date: Wed, 23 Jan 2019 10:28:53 +0100

Hi Chris,

thanks for the patch!

> * gnu/packages/patchutils.scm (patchwork): New variable.
[…]
> +         (replace 'check
> +           (lambda* (#:key tests? #:allow-other-keys)
> +             (or (not tests?)
> +                 (begin
> +                   (setenv "DJANGO_SETTINGS_MODULE" "patchwork.settings.dev")
> +                   (invoke
> +                    "python" "-Wonce" "./manage.py" "test" "--noinput")
> +                   #t))))

Maybe write this as

         (replace 'check
           (lambda* (#:key tests? #:allow-other-keys)
             (when tests?
               (setenv "DJANGO_SETTINGS_MODULE" "patchwork.settings.dev")
               (invoke
                "python" "-Wonce" "./manage.py" "test" "--noinput"))
             #t))

> +         (replace 'install
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let ((out (assoc-ref outputs "out"))) […]

This phase might be less verbose if you let-bound the result of
(site-packages inputs outputs) at the beginning.  It would also be good
if there were more comments about what’s going on.  It’s not all obvious
(e.g. why “lib” is copied to “docs”).

> +               (simple-format #t "replacing template pwclient symlink")

Use “display” instead of “simple-format #t”?

> +         (add-after 'install 'install-hasher
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out")))
> +               (chmod (string-append (site-packages inputs outputs)
> +                                     "/patchwork/hasher.py")
> +                      #o555)
> +               (symlink (string-append (site-packages inputs outputs)
> +                                       "/patchwork/hasher.py")
> +                        (string-append out "/bin/hasher")))
> +             #t))

Here also consider simplifying with let.

> +         ;; Create a patchwork specific version of Django's command line admin
> +         ;; utility.
> +         (add-after 'install 'install-patchwork-admin
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out")))
> +               (mkdir-p (string-append out "/bin"))
> +               (call-with-output-file (string-append out "/bin/patchwork-admin")
> +                 (lambda (port)
> +                   (display "#!/usr/bin/env python3

Should this really say “#!/usr/bin/env python3”?

--
Ricardo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.