[Bug 210007] Review Request: libtune - standard API to access the kernel tunables

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: libtune - standard API to access the kernel tunables


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





------- Additional Comments From nadia.derbey at bull.net  2008-01-08 06:46 EST -------
Konrad,

Sorry for the late answer!

(In reply to comment #54)
> Per http://fedoraproject.org/wiki/Packaging/ReviewGuidelines:
> 
> 1).
<snip>
> Please release upstream the newer version (0.14-2) and as well in the .spec file
> reference the %{name}-%{version}-%{release}.tar.bz2

Fixed.
Actually, didn't use the release tag in the source name: only the release number
is referenced (%{release} contains the dist tag and I'd like the source.tar.bz2
to be independant from the distro name). 

> 
> 2). MUST: In the vast majority of cases, devel packages must require the base
> package using a fully versioned dependency: Requires: %{name} =
> %{version}-%{release} 
> 
> Currently the .spec file has:
> Requires: %{name} = %{version}
> 
> That should be:
> Requires: %{name} = %{version}-%{release}
> 

Fixed.

(In reply to comment #55)
> There is also an inconsistent usage of spaces.

Fixed.

(In reply to comment #56)
> Please also add this for the main package and -devel package
> Requires(post): /sbin/ldconfig
> Requires(postun): /sbin/ldconfig
> 
> As you are using ldconfig in the post and postun section. 

Only added this Requires: to the post section for the devel package: in the
other cases, rpmbuild generates it automatically (checked through the rpmbuild
traces), so I thought it was not necessary.

> 
> Furthermore for the -devel, also add:
> Requires(post): /sbin/chkconfig
> Requires(postun): /sbin/chkconfig
> Requires(post): /usr/lib/lsb/install_initd
> Requires(postun): /usr/lib/lsb/remove_initd
> 
> As you are using /sbin/chkconfig and /usr/lib/lsb/install_initd in post, and 
> postun sections.

Well, only added the chkconfig Requires: install/remove_initd is called only if
present on the system we are installing on/deinstalling from, i.e. if the system
is LSB compliant. If it is not, chkconfig is called. So chkconfig is THE one we
need.

(In reply to comment #57)
> You also need to update your %post, %preun and %postun to take advantage of a
> user updating the package.

Good catch: actually, I had never tested the upgrade part of it and it was not
correct!
Fixed.

New links:
SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-14-3/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.14-3.fc8.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.