[Bug 1134340] Review Request: python-unp - unp is a command line tool that can unpack archives easily

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

 



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



--- Comment #2 from Matej Stuchlik <mstuchli@xxxxxxxxxx> ---
(In reply to Eduardo Mayorga from comment #1)
> Issues
> ======
> - Latest upstream release is now 0.3

Done, mitsuhiko is flying.

> - You must include a license file if upstream decides to not distribute it
> in the sources. You can pull the license text copy from Github repository,
> and you should contact upstream to get this mistake corrected.
>   See:
> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

License included, I'll let upstream know later today.

> - Since this is not a Python addon module, unp would be a better name.

I was operating under the assumption that someone may want to use it as a
module, but I suppose that's not really the case, so... Done. Given this, I
will also drop the python3 subpackage, and will use python3 in the base
package.

> - Upstream source tarball timestamps are not preserved. Please download it
> using a client using proper options.
>   See: https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps

Done.

> - Fix the shebangs for both Python 2 and Python 3 scripts. This can solve
> the problem:
>   %if 0%{?with_python3}
>   rm -rf %{py3dir}
>   cp -a . %{py3dir}
>   find %{py3dir} -name '*.py' | xargs sed -i '1s|^#!python|#!%{__python3}|'
>   %endif # with_python3
> 
>   find -name '*.py' | xargs sed -i '1s|^#!python|#!%{__python2}|'

This shouldn't be necessary, setuptools take care of it, check the rpm archive.

> - Change BR python-devel to python2-devel.

Done.

> - There's a test suite that you should run in %check.

Could you please point me to the test suite? I can't seem to find it.

> - The summary can be improved. "unp is"... is superfluous, simple leave it
> as: A command line tool that can unpack archives easily

Done.

> - python-setuptools is not a runtime dependency, so you can drop this
> Requires.

I don't believe you're correct here, from /usr/bin/unp:

from pkg_resources import load_entry_point

pkg_resources is provided by setuptools.

> - We only have click 2.4 in F20, so this will only work in F21+. When I run
> it in my F20:
> [makerpm@localhost 1134340-python-unp]$ unp
> Traceback (most recent call last):
>   File "/bin/unp", line 5, in <module>
>     from pkg_resources import load_entry_point
>   File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 2797, in
> <module>
>     parse_requirements(__requires__), Environment()
>   File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 576, in
> resolve
>     raise DistributionNotFound(req)
> pkg_resources.DistributionNotFound: click>=3.0

I am aware of that. The click maintainer isn't planning on updating it in f20,
so I'll only add unp to f21+.

New Spec URL: https://mstuchli.fedorapeople.org/unp.spec
New SRPM URL: https://mstuchli.fedorapeople.org/unp-0.3-1.fc20.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
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]