[Bug 1421245] Review Request: libcrush - C library to control placement in a hierarchy

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

 



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

Michael Schwendt <bugs.michael@xxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|177841 (FE-NEEDSPONSOR)     |



--- Comment #5 from Michael Schwendt <bugs.michael@xxxxxxx> ---
Since you're sponsored already despite the packaging mistakes, there's no need
to block the FE-NEEDSPONSOR tracker.


SRPM URL -> 404 not found


> %define libname %{name}1
> Name:           libcrush

Please don't do that. It's a confusing mess. You're blocking the libcrush
namespace for dist git, the src.rpm and the spec file, too. You also miss
automatic arch-specific Provides. And worse, your virtual package naming is
half-hearted as you build libcrush-devel and libcrush1. If there ever will be a
separate "libcrush1" package, create it when necessary, and make that one
provide a libcrush1-devel package, too.


> %package devel
> Summary:        C library to control placement in a hierarchy development files
> Requires:       %{libname} = %{version}

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

​
> %description

Descriptions of both packages are the same. You should be explicit about the
-devel package only containing files needed for development.


> # FIXME: you should use %%cmake macros
> cmake . \

Indeed. The better comment would explain why %cmake isn't used or whether it
cannot be used [yet].


> %{_datadir}/pkgconfig/libcrush.pc

Wrong installation directory for arch-specific .pc files.
Wrong libdir definition for 64-bit targets.
Is relinking with libm necessary?
Adding /usr/include/crush to headers search path bears risk, since that
directory includes headers with very generic names. Preferably, API users and
the library headers themselves would include all libcrush headers via
<crush/foo.h> from standard search path.


> %changelog

Even for the initial package, there ought to be a first entry.
https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor
-- 
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
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




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