[Bug 1275517] Review Request: python-wand - python bindings for ImageMagick

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

 



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

Rex Dieter <rdieter@xxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |rdieter@xxxxxxxxxxxx
           Assignee|nobody@xxxxxxxxxxxxxxxxx    |rdieter@xxxxxxxxxxxx
              Flags|                            |fedora-review?



--- Comment #2 from Rex Dieter <rdieter@xxxxxxxxxxxx> ---
Welcome Dennis, I'd be happy to help review this (and sponsor you).

Per Randy's comment's:

0.  this is actually ok, it's already wrapped inside the conditional
%if %{with python3}
...
%endif

1.  MUST: however, putting runtime dep here is wrong, you really want the 
Requires: ImageMagick
down under
%package -n python3-wand

for cleanliness, and since this subpkg is already wrapped by %if %{with
python3} itself, you could move both the runtime and build python3-related deps
here, to look something like:

%if %{with python3}
%package     -n python3-wand
Summary:        Ctypes-based simple MagickWand API binding for Python
BuildRequires:  python3-devel
BuildRequires:  python3-setuptools
Requires:       ImageMagick
%description -n python3-wand
Wand is a ctypes-based simple ImageMagick binding for Python. It doesn't cover
all functionalities of MagickWand API currently. It works on Python 2.6, 2.7,
3.2, 3.3, and PyPy.
%endif


naming: ok

2. SHOULD consider omitting from %description the imo mostly-needless phrase:
"It works on Python 2.6, 2.7, 3.2, 3.3, and PyPy."

sources: kinda ok, but

3.  SHOULD see
https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Git_Tags
about how to rename the source tarball, to use something like:
https://github.com/dahlia/wand/archive/%{version}.tar.gz#/%{name}-%{version}.tar.gz
instead

license: ok

macros: ok, mostly...

4.  SHOULD omit from %install:
rm -rf $RPM_BUILD_ROOT
it is deprecated and no longer needed.

scriptets: ok (n/a)


Please fix all MUST items, and consider addressing SHOULD's, and we should be
good to go.

-- 
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]