Re: [PATCH] Add tinycm.

[Leo Famulari <leo@famulari.name>]

[-- Attachment #1: Type: text/plain, Size: 3066 bytes --]

On Wed, Sep 21, 2016 at 12:07:22PM +0000, ng0 wrote:
> This adds tinycm. But now that I am finished, I think this is not what I
> wanted, I wanted http://tinycm.slitaz.org/ . I assumed without looking
> at the source that this is just a fork.
> Well, no damage in more packages I'd say.

Thanks for this patch series!

> 

> Subject: [PATCH 1/8] gnu: Add python-boolexp.
> 
> * gnu/packages/python.scm (python-boolexp): New variable.

> +(define-public python2-boolexp
> +  (let ((boolexp (package-with-python2 python-boolexp)))
> +    (package (inherit boolexp)
> +      (native-inputs
> +       `(("python2-setuptools" ,python2-setuptools))))))

This won't work in all cases. You have to use the python2-variant system
when python-2 versions of packages have different inputs from the
python-3 version. Can you update the patch? See commit
bd74be7b8c6f3df34d8556f55ab9ef7411c07933 for an example.

Some more of the patches have this issue. I won't mention it again as I
remove them.

> Subject: [PATCH 3/8] gnu: Add python2-ruamel.ordereddict
> 
> * gnu/packages/python.scm (python2-ruamel.ordereddict): New variable.

> +    ;; XXX: https://bitbucket.org/ruamel/ordereddict/issues/2
> +    (arguments
> +     `(#:python ,python-2))

Please add a line to the comment that says something like "Only Python 2
is supported", so that the hyperlink has some context.

> Subject: [PATCH 4/8] gnu: Add python-distro.
> 
> * gnu/packages/python.scm (python-distro): New variable.

`guix lint` should suggest some changes to the synopsis and description.

> Subject: [PATCH 5/8] mailmap: Add new address for ng0.

LGTM but I have no experience with this subject.

> Subject: [PATCH 6/8] gnu: Add python-typing.
> 
> * gnu/packages/python.scm (python-typing): New variable.

LGTM

> Subject: [PATCH 7/8] gnu: Add python-ruamel.yaml.
> 
> * gnu/packages/python.scm (python-ruamel.yaml): New variable.

> +    (arguments
> +     `(;; Otherwise it complains that you have to do "pypi install ."
> +       #:configure-flags (list "--single-version-externally-managed"
> +                               "--record=ruamel.yaml.txt")))

Is the '--record' line also described by the comment about "pypi
install"? If not please add a comment that explains it.

> +    (inputs
> +     `(("python-typing" ,python-typing)))

> +(define-public python2-ruamel.yaml
> +  (package (inherit (package-with-python2
> +                     (strip-python2-variant python-ruamel.yaml)))
> +    (name "python2-ruamel.yaml")
> +    (inputs
> +     `(("python2-ruamel.ordereddict" ,python2-ruamel.ordereddict)
> +       ("python2-typing" ,python2-typing)))
> +    (native-inputs
> +     `(("python2-setuptools" ,python2-setuptools)))))

You'll need to add a (properties) field to ensure that the
python2-variant is used. Also, you don't have to specify python2-typing;
you can append to the inherited inputs that are transformed by
package-with-python2. See 0adc21c2933fbe47ee37d90acdb4ad8cbc60ed69 for
an example.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]