Thanks for the patch set!
I haven't properly tested the package yet. The following are just myinitial reactions and questions. This patch review will take a fewiterations. Do bear with me.
Toggle quote (8 lines)
> Tryton has modules and without any module packaged, it will do nothing>> But at least you can launch it and test it, you can use it for packkaging> the missing modules.>> Also a service would be useful. But in order to write a service, the server> packkage has to be in already.
Agreed.
Toggle quote (2 lines)
> This is supposedly the basis for GNUealth, a notable GNU project
GNU Health usually lags behind the latest Tryon, and currently runs onTryton 3.8. We will have to create a package for Tryton 3.8 aswell. This can just inherit from the latest tryton package, and modifyonly the `version' and `source' fields. Could you do this?
Toggle quote (5 lines)
> From e42a727312a454aeb19e07cfec6cbb03fe18e183 Mon Sep 17 00:00:00 2001> From: humanitiesNerd <catonano@gmail.com>> Date: Tue, 28 Mar 2017 12:25:06 +0200> Subject: [PATCH 1/5] gnu: Add python-sql python2-sql.
It is enough to mention only python-sql here.
Toggle quote (2 lines)
> * gnu/packages/python.scm (python-sql python2-sql): New variables.
Please put a comma between python-sql and python2-sql.
Toggle quote (5 lines)
> +(define-public python-sql> + (package> + (name "python-sql")> + (version "0.8")
The latest version of python-sql is 0.9.
Toggle quote (4 lines)
> + (uri (pypi-uri> + "python-sql"> + version))
Could you put these on the same line?
Toggle quote (12 lines)
> +(define-public python-genshi> + (package> + (name "python-genshi")> + (version "0.7")> + (source> + (origin> + (method url-fetch)> + (uri (string-append> + "https://ftp.edgewall.org/pub/genshi/Genshi-"> + version> + ".tar.gz"))
Please put version ".tar.gz" on the same line.
Toggle quote (14 lines)
> + (patches> + (search-patches> + ;; The first 4 patches are in the master branch upstream.> + ;; see this as a reference https://genshi.edgewall.org/ticket/582> + ;; The last 2 are NOT in any branch.> + ;; They were sent as attachments to a ticket opened at> + ;; https://genshi.edgewall.org/ticket/602#no1> + "python-genshi-stripping-of-unsafe-script-tags-Python-3.4.patch"> + "python-genshi-Disable-the-speedups-C-extension-on-CPython-3.3-sinc.patch"> + "python-genshi-isstring-helper.patch"> + "python-genshi-Add-support-for-Python-3.4-AST-support-for-NameConst.patch"> + "python-genshi-fixing-the-tests-on-python35.patch"> + "python-genshi-buildable-on-python27-too.patch"))
Why do we need these patches? Is the release tarball not sufficient?
Toggle quote (4 lines)
> + (propagated-inputs> + `(("lxml" ,python2-lxml)> + ("genshi" ,python2-genshi)))
Please put the full names of these inputs -- I mean "python-lxml"instead of "lxml", "python-genshi" instead of "genshi", and so on.
Toggle quote (4 lines)
> +(define-public python-trytond> + (package> + (name "python-trytond")
As far as I understand, trytond is an application, not a pythonlibrary. Only python libraries should have the "python-" prefix. So,this package would just be called "trytond".
Toggle quote (2 lines)
> + (version "4.2.3")
The latest version of tryton is 4.4.
Toggle quote (8 lines)
> + (source> + (origin> + (method url-fetch)> + (uri (pypi-uri> + "trytond"> + version> + ".tar.gz"))
We should use the tarballs available on the tryton website.https://downloads.tryton.org/4.4/ Toggle quote (8 lines)
> + (arguments> + `(#:phases> + (modify-phases %standard-phases> + (add-before 'check 'preparations> + (lambda* _> + ;; this is used in the tests> + (setenv "DB_NAME" ":memory:"))))))
Though this is shorter, I think it would be clearer to replace the`check' phase altogether.
Toggle quote (13 lines)
> + (propagated-inputs> + `(("polib" ,python-polib)> + ("dateutil" ,python-dateutil)> + ("werkzeug" ,python-werkzeug)> + ("wrapt" ,python-wrapt)> + ("python-sql" ,python-sql)> + ("genshi" ,python-genshi)> + ("relatorio" ,python-relatorio)> + ("lxml" ,python-lxml)> + ;; there's no pyton-mysql in Guix right now> + ;; so psycopg (postgresql) only for now> + ("psycopg" ,python-psycopg2)))
If trytond is only an application, these can just be `inputs', not`propagated-inputs'. For applications, the python build system wraps theexecutables with the correct PYTHONPATH environment variable.
Toggle quote (2 lines)
> + (license license:lgpl3)))
Tryton is GPL3.
Toggle quote (3 lines)
> +(define-public python2-trytond> + (package-with-python2 python-trytond))
No need for python2-trytond if trytond is just an application.
Toggle quote (6 lines)
> +;; this depends on pygtk that is available or python@2 only> +(define-public python2-tryton> + (package> + (name "python2-tryton")> + (version "4.2.4")
Latest version if 4.4
Toggle quote (5 lines)
> + (uri (pypi-uri> + "tryton"> + version> + ".tar.gz"))
We should use the tarballs available on the tryton website.https://downloads.tryton.org/4.4/ Toggle quote (5 lines)
> + (propagated-inputs> + `(("chardet" ,python2-chardet)> + ("dateutil" ,python2-dateutil)> + ("pygtk" ,python2-pygtk)))
For an application, these can just be `inputs'.