[Bug 691114] Review Request: python-msgpack - A MessagePack (de)serializer

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=691114

Martin Gieseking <martin.gieseking at uos.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |martin.gieseking at uos.de

--- Comment #3 from Martin Gieseking <martin.gieseking at uos.de> 2011-04-13 15:12:26 EDT ---
David, good job. Your informal review looks fine. When reviewing Python
packages, please also check the additional items listed here:
http://fedoraproject.org/wiki/Packaging:Python#How_to_package

A few additional comments:

(In reply to comment #1)
> It'd be good if the nosetests provided were run in %check. I had a quick
> attempt at making them run but didn't get it working.

Right. Running the tests would be nice if they are up-to-date with the module
sources. You can execute them with 
   PYTHONPATH="%{buildroot}%{python_sitearch}" nosetests -w test
in a %check section.
Unfortunately, they currently crash with a segfault. Fabian, please ask
upstream whether this is a bug in the Python module or a test-only issue.

The permissions of the .so file should be set to 755 (see rpmlint output):

$ rpmlint /var/lib/mock/fedora-14-x86_64/result/*.rpm
python-msgpack.src: W: spelling-error Summary(en_US) serializer -> serialize,
serializes, serialized
python-msgpack.x86_64: W: spelling-error Summary(en_US) serializer ->
serialize, serializes, serialized
python-msgpack.x86_64: W: private-shared-object-provides
/usr/lib64/python2.7/site-packages/msgpack/_msgpack.so _msgpack.so()(64bit)
python-msgpack.x86_64: E: non-standard-executable-perm
/usr/lib64/python2.7/site-packages/msgpack/_msgpack.so 0775L
3 packages and 0 specfiles checked; 1 errors, 3 warnings.


> The README states that Cython is required to build msgpack, but the try/except
> ImportError in setup.py suggests otherwise. I successfully built the package
> both with and without Cython...

The tarball contains the cython-created .c file. So there's no need recreate
it.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.