From: Christopher Baines <mail@cbaines.net>
To: Ricardo Wurmus <rekado@elephly.net>
Cc: 33185@debbugs.gnu.org
Subject: [bug#33185] [PATCH v2 1/2] gnu: Add patchwork.
Date: Fri, 25 Jan 2019 21:04:46 +0000 [thread overview]
Message-ID: <875zucih3l.fsf@cbaines.net> (raw)
In-Reply-To: <87imyf7mei.fsf@elephly.net>
[-- Attachment #1: Type: text/plain, Size: 3322 bytes --]
Ricardo Wurmus <rekado@elephly.net> writes:
> Hi Chris,
>
> thanks for the patch!
And thanks for your feedback :) I've sent some updated patches.
>> * 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))
Yep, I've updated it to use when now.
>> + (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”).
I've used let more now, and tried to be a bit more descriptive in the
comments. I'll reconfigure the patchwork instances I host at some point
and hopefully add more detail then.
>> + (simple-format #t "replacing template pwclient symlink")
>
> Use “display” instead of “simple-format #t”?
That was just a debugging thing, I've removed it.
>> + (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.
Yep, I've used let here more now.
>> + ;; 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”?
So this was fine, as it was replaced in a future stage. But yes, it's
probably better to use a more appropriate value that isn't changed later
on. I've updated this now.
So yes, I've sent some updated patches, still a few niggles, but they're
getting better.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]
next prev parent reply other threads:[~2019-01-25 21:05 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 ` [bug#33185] [PATCH v2 1/2] gnu: " Ricardo Wurmus
2019-01-25 21:04 ` Christopher Baines [this message]
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=875zucih3l.fsf@cbaines.net \
--to=mail@cbaines.net \
--cc=33185@debbugs.gnu.org \
--cc=rekado@elephly.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).