Re: [PATCH] Add Gajim.

[Ricardo Wurmus <rekado@elephly.net>]

Mark H Weaver <mhw@netris.org> writes:
>> * gnu/packages/python.scm (python-ipaddress, python2-ipaddress): New
>>   variables.

[...]

>
> Hmm, but we already have Python 3.4, and this package seems to be built
> with it, so I'm confused :)
>
> In any case, it would be good to describe what the ipaddress module does
> here, instead of simply saying that it's a backport.

Oh, right.  I must have been on autopilot.  I have removed
‘python-ipaddress’ and added a proper description for
‘python2-ipaddress’.

>> +    (description
>> +     "Pretend is a library to make stubbing with Python easier.  Stubbing is a
>> +technique for writing tests.  You may hear the term mixed up with mocks,
>> +fakes, or doubles. Basically a stub is an object that returns pre-canned
>
> Two spaces between sentences, please.  Otherwise looks good to me.

Sorry.  Fixed.

>> * gnu/packages/python.scm (python-cryptography-vectors,
>>   python2-cryptography-vectors): New variables.

[...]

>> +    ;; Distributed under either BSD-3 or ASL2.0
>> +    (license bsd-3)))
>
> So shouldn't it be (license (list bsd-3 asl2.0)) ?

Right.  Fixed.

>> From 678b5402e9a226383abbd5d2e560d3f609d719e6 Mon Sep 17 00:00:00 2001
>> From: Ricardo Wurmus <rekado@elephly.net>
>> Date: Mon, 21 Sep 2015 22:46:11 +0200
>> Subject: [PATCH 07/11] gnu: Add python-cryptography.
>>
>> * gnu/packages/python.scm (python-cryptography, python2-cryptography):
>>   New variables.

[...]

>> +    ;; Distributed under either BSD-3 or ASL2.0
>> +    (license bsd-3)))
>
> Ditto.

Fixed.

>> * gnu/packages/gnupg.scm (python-gnupg, python2-gnupg): New variables.

[...]

>> +                    (setenv "GPGBINARY" (which "gpg"))
>> +                    (setenv "USERNAME" "guixbuilder")
>
> Why "guixbuilder"?  I don't think that name has any significance within
> the build environment, does it?

The name is not important, but USERNAME must be set.

>> +                    ;; The doctests are extremely slow and sometimes time out,
>> +                    ;; so we disable them.
>> +                    (zero? (system* (which "python")
>
> This could just be "python" (without the call to 'which'), I think.
> Ditto for GPGBINARY, although I guess it's not important.

You are right.  I changed it.

>> +(define-public gajim
>> +  (package
>> +    (name "gajim")
>> +    (version "0.16.3")
>> +    (source (origin
>> +              (method url-fetch)
>> +              (uri (string-append "https://gajim.org/downloads/"
>> +                                  "0.16" "/gajim-" version ".tar.bz2"))
>
> Please use (version-major+minor version) instead of "0.16" here.

Yes, I missed this.  Thanks.

>> +              (sha256
>> +               (base32
>> +                "05a59hf9wna6n9fi0a4bhz1hifqj21bwb4ff9rd0my23rdwmij51"))))
>> +    (build-system gnu-build-system)
>> +    (arguments
>> +     `(#:phases
>> +       (modify-phases %standard-phases
>> +         (add-after 'install 'wrap-program
>> +          (lambda* (#:key inputs outputs #:allow-other-keys)
>
> 'inputs' is unused here.

Removed.

>> +            ;; Make sure all Python scripts run with the correct PYTHONPATH.
>> +            (let* ((out (assoc-ref outputs "out"))
>> +                   (path (getenv "PYTHONPATH")))
>
> This could be 'let' instead of 'let*'.

Fixed.

> Otherwise looks good.

Thanks for the review!

I’ve had to slightly modify a couple of the python2-* package
definitions in addition to the above changes as a result of removing
‘python-ipaddress’.  I’ll push the commits in a moment.

~~ Ricardo