[Bug 833853] Review Request: libkolab - Kolab Format Handler library

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

 



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

--- Comment #4 from Jeroen van Meeuwen <kanarip@xxxxxxxxxxx> ---
(In reply to comment #3)
> Naming: ok
> 
> 1. MUST sources: Not ok, not verifiable.  no upstream 0.3 tarball (yet?)
> 6d9a81c318193c2d3ec580b73ffa8886  libkolab-0.3.tar.gz
> looks like this may be a snapshot, so providing a recipe to create the
> tarball would be enough.
> 

Changed this to:

# From
http://git.kolab.org/libkolab/snapshot/54d11d067951ddedd8b28aa7eb54e4f68bc81218.tar.gz
Source0:        http://git.kolab.org/%{name}/snapshot/%{name}-%{version}.tar.gz

for the time being, while this still comes straight from the master branch.

> SHOULD: I'd suggest using
> URL:  http://git.kolab.org/libkolab
> instead of http://kolab.org/
> 

I'll enlist some help to create some sort of http://kolab.org/software/libkolab
page, but for now I've changed the URL to the GIT repository browse location as
suggested.

> SHOULD: remove rpm cruft like Group:, BuildRoot, %defattr
> 

Done.

> SHOULD: remove extraneous cmake qt-devel deps, like
> BuildRequires:  cmake >= 2.6
> BuildRequires:  qt-devel >= 4.7
> (and similar install-time deps in the -devel subpkg), these are pulled in
> implicitly via kdepimlibs-devel already
> 

Done.

> SHOULD: track lib soname's explicitly, use
> %{_libdir}/libkolab.so.0
> instead of overly broad
> %{_libdir}/libkolab.so.*
> 

Done.

> SHOULD: these cmake directives are extraneous and already included in
> fedora's default %{cmake} macro, so can be removed:
>     -DCMAKE_SKIP_RPATH=ON \
>     -DCMAKE_INSTALL_PREFIX=%{_prefix} \
>     -DLIB_INSTALL_DIR=%{_libdir} \
> 

Removed.

> 2. MUST: scriptlets NOT ok, missing:
> %post -p /sbin/ldconfig
> %postun -p /sbin/ldconfig
> 

Included.

> Licensing: OK
> confirmed a mixture of LGPLv2+ and LGPLv3+, though no LICENSE or COPYING
> file is provided (another SHOULD nice-to-have fix)
> 

Inclusion issue logged as https://bugzilla.kolabsys.com/show_bug.cgi?id=859

> 3. -devel pkg MUST have and arch'd dep on the main pkg, using
> Requires: %{name}%{?_isa} = %{version}-%{release}
> 

Done.

> Address at least MUST items 1-3, and looks like we have a winner.

Thanks for the review Rex.

New SPEC: http://git.kolabsys.com/rpm/libkolab/plain/libkolab.spec
New SRPM:
http://mirror.kolabsys.com/pub/fedora/kolab-3.0/f17/development/SRPMS/libkolab-0.3-5.fc17.kolab_3.0.src.rpm

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