[Bug 810376] Review Request: pypng - Python PNG encoder/decoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=810376

--- Comment #3 from Kalev Lember <kalevlember@xxxxxxxxx> 2012-04-06 10:20:47 EDT ---
(In reply to comment #2)
> I have a preference for "pypng", because people tend to call it that

Fair enough.


The main issue I see with this package is that its licensing is unclear.
code/png.py appears to be MIT-licensed, but the rest of the code files don't
have any licensing information. There's also no readme to clear this up.

Could you ask upstream to clarify licensing and add license headers to code
files?


Also some comments about the spec file:

RPM doesn't have automatic python dep extraction, so this package will have to
manually specify Requires for other python modules it uses. numpy at least is
missing from Requires, perhaps something else as well.


> %{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib())")}

python_sitelib is automatically defined by rpm macros in all supported Fedora
releases so no need to define it in the spec file.
https://fedoraproject.org/wiki/Packaging:Python#Macros


> Source0: http://pypng.googlecode.com/files/pypng-0.0.12.tar.gz

Use the %{version} macro here. With this, when updating to a new upstream
release, you'll only need to update the Version: tag and the source URL won't
need updating each time.


> %build
> # Remove CFLAGS=... for noarch packages (unneeded)
> CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build

This _is_ a noarch package, so CFLAGS aren't needed here.


> %install
> rm -rf $RPM_BUILD_ROOT

The rm -rf isn't needed with the version of rpmbuild in supported Fedora
releases.

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]