[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

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

 



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



--- Comment #2 from José Matos <jamatos@xxxxxxxx> ---
(In reply to Igor Gnatenko from comment #1)
> I will review it today/tomorrow.

Thank you. :-)

> Initial comments:
> 
> > BuildRequires:  python-setuptools
> BuildRequires:  python2-setuptools

Right, I used pyp2rpm to generate the first version and this detail was lost in
the revision. I will fix it in the next revision.

> > %{python2_sitelib}/%{pypi_name}
> %{python2_sitelib}/%{pypi_name}/

Why do we need the trailing slash?

IIRC in %files

mydir

is the same as

%dir mydir
mydir/*

I admit that my memory could be failing. :-)

> > %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info
> %{python2_sitelib}/%{pypi_name}-*.egg-info/
> or
> %{python2_sitelib}/%{pypi_name}-%{version}-py%{python2_version}.egg-info/
> (note trailing slash)
> 
> .. same for python3

I will change to the * version or else it will fail at python 3.10. :-)

> * I think it has missing Requires: pythonX-pillow

No it does not. I looked into the code, it mainly imports struct and io that
are in the standard library.

It is supposed to work with pillow and thus the tests but it does not require
it.

> * I would not put tests out of tree, as if something changes on github you
> will not notice.
> * I would just build from git tag (and write comment that this commit is
> actually version X.Y.Z) - https://github.com/hMatoba/Piexif/issues/23

I was expecting to have the nice upstream to release a new version soon. We can
always hope.

> * Don't duplicate %description, define it just once

I saw a tip from you recently but I forgot it. How do you suggest to have a
single description?

> * Move BuildRequires under subpackages for better look&feel

Fair request. :-)

-- 
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://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx




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