unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Ricardo Wurmus <rekado@elephly.net>
To: Christopher Baines <mail@cbaines.net>
Cc: 33185@debbugs.gnu.org
Subject: [bug#33185] [PATCH v2 1/2] gnu: Add patchwork.
Date: Wed, 23 Jan 2019 10:28:53 +0100	[thread overview]
Message-ID: <87imyf7mei.fsf@elephly.net> (raw)
In-Reply-To: <20190122220928.17927-1-mail@cbaines.net>


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

  parent reply	other threads:[~2019-01-23  9:43 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-28  9:21 [bug#33185] [PATCH 0/7] Add patchwork package and service Christopher Baines
2018-10-28  9:26 ` [bug#33185] [PATCH 1/7] gnu: Add python-jsmin Christopher Baines
2018-10-28  9:26   ` [bug#33185] [PATCH 2/7] gnu: Add python-slimit Christopher Baines
2018-11-19 16:30     ` Ludovic Courtès
2018-11-20 19:58       ` Christopher Baines
2018-10-28  9:26   ` [bug#33185] [PATCH 3/7] gnu: Add python-django-pipeline Christopher Baines
2018-11-19 16:30     ` Ludovic Courtès
2018-10-28  9:26   ` [bug#33185] [PATCH 4/7] gnu: Add python-django-jinja Christopher Baines
2018-10-28  9:27   ` [bug#33185] [PATCH 5/7] gnu: Add python-django-debug-toolbar Christopher Baines
2018-10-28  9:27   ` [bug#33185] [PATCH 6/7] gnu: Add patchwork Christopher Baines
2018-10-28  9:27   ` [bug#33185] [PATCH 7/7] services: " Christopher Baines
2018-11-19 16:29   ` [bug#33185] [PATCH 1/7] gnu: Add python-jsmin Ludovic Courtès
2018-11-04 10:44 ` Christopher Baines
2018-11-04 10:44   ` [bug#33185] [PATCH 2/7] gnu: Add python-slimit Christopher Baines
2018-11-04 10:44   ` [bug#33185] [PATCH 3/7] gnu: Add python-django-pipeline Christopher Baines
2018-11-04 10:44   ` [bug#33185] [PATCH 4/7] gnu: Add python-django-jinja Christopher Baines
2018-11-19 16:33     ` Ludovic Courtès
2018-11-20 20:02       ` Christopher Baines
2018-11-04 10:44   ` [bug#33185] [PATCH 5/7] gnu: Add python-django-debug-toolbar Christopher Baines
2018-11-19 16:33     ` Ludovic Courtès
2018-11-20 20:03       ` Christopher Baines
2018-11-04 10:44   ` [bug#33185] [PATCH 6/7] gnu: Add patchwork Christopher Baines
2018-11-04 19:09     ` swedebugia
2019-01-22 22:36       ` Christopher Baines
2018-11-19 16:37     ` Ludovic Courtès
2019-01-22 22:31       ` Christopher Baines
2018-11-04 10:44   ` [bug#33185] [PATCH 7/7] services: " Christopher Baines
2018-11-04 19:10     ` swedebugia
2018-11-19 16:42       ` Ludovic Courtès
2018-11-20 18:22         ` Christopher Baines
2018-11-20 18:06       ` Christopher Baines
2019-01-22 22:09 ` [bug#33185] [PATCH v2 1/2] gnu: " Christopher Baines
2019-01-22 22:09   ` [bug#33185] [PATCH v2 2/2] services: " Christopher Baines
2019-01-22 22:40     ` Christopher Baines
2019-01-23  9:28   ` Ricardo Wurmus [this message]
2019-01-25 21:04     ` [bug#33185] [PATCH v2 1/2] gnu: " Christopher Baines
2019-01-25 21:00 ` [bug#33185] [PATCH v3 " Christopher Baines
2019-01-25 21:00   ` [bug#33185] [PATCH v3 2/2] services: " Christopher Baines
2019-05-03 19:16 ` [bug#33185] [PATCH 0/7] Add patchwork package and service Christopher Baines
2019-05-03 19:30   ` [bug#33185] [PATCH 1/3] services: Add getmail Christopher Baines
2019-05-03 19:30     ` [bug#33185] [PATCH 2/3] gnu: Add patchwork Christopher Baines
2019-05-03 19:30     ` [bug#33185] [PATCH 3/3] services: " Christopher Baines
2019-05-31 19:43   ` bug#33185: [PATCH 0/7] Add patchwork package and service Christopher Baines

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87imyf7mei.fsf@elephly.net \
    --to=rekado@elephly.net \
    --cc=33185@debbugs.gnu.org \
    --cc=mail@cbaines.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).