Bug 225913 - Merge Review: irqbalance
Summary: Merge Review: irqbalance
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 19:07 UTC by Nobody's working on this, feel free to take it
Modified: 2009-07-31 18:05 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-07-31 18:05:39 UTC
Type: ---
Embargoed:
lemenkov: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 19:07:19 UTC
Fedora Merge Review: irqbalance

http://cvs.fedora.redhat.com/viewcvs/devel/irqbalance/
Initial Owner: nhorman

Comment 1 Peter Lemenkov 2009-07-27 19:22:08 UTC
I'll review it.

Comment 2 Neil Horman 2009-07-27 19:40:15 UTC
I'm sorry, whats the purpose of this review?

Comment 3 Jason Tibbitts 2009-07-27 19:46:22 UTC
All packages which were in Fedora Core at the time of the merge between Core and Extras need to be reviewed for compliance with the packaging guidelines.  Tickets were opened 2.5 years ago for this, but progress has been slow.

Comment 4 Neil Horman 2009-07-27 19:54:03 UTC
ah, that explains my confusion.  Thanks!

Comment 5 Peter Lemenkov 2009-07-28 10:07:57 UTC
The package is in so sorrow state, so it doesn't even builds on my machine.
OK,here is a list of issues:

* The package doesn't honours optflags. Moreover, it adds -Os ("optimize foir size") parameter to the list of GCC keys. I'm not sure whether it conflicts with our -O2, since I'm not keen in GCC tweaking. However, it may be easily removed at %prep stage (adding of something like that - sed -i s/-Os//g %{name}-%{version}/Makefile )

* ExclusiveArch directive should use macro %{ix86} instead of "i386 i586"

* Use Requires(Pre,Preun) instead of Prereq

* Unneeded Requires - /sbin/service

* BuildRoot MUST contain at least %{name}, %{version} and %{release}. See recommended values here:

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

Although this directive is obsolete since F-10, I think it's a good idea to manually specify it, just in case, that the only spec will be used both in Fedora and in RHEL/EPEL/Whatever.

* No parallel make. See below my suggestions, regarding %build section.

* The long line above should be shortened ( rm -rf $RPM_BUILD_ROOT will be enough):
[ "$RPM_BUILD_ROOT" != "/" ] && [ -d $RPM_BUILD_ROOT ] && rm -rf $RPM_BUILD_ROOT; 

* rm -rf $RPM_BUILD_ROOT must be in %install section, and not in %build

* Both %build and %install sections are highly exaggerated. You don't need to manually create all these directories - "install" utility can do all this work for us.

* inconsistent use of macros. Sometimes you're using $RPM_BUILD_ROOT and sometimes %{buildroot}.

* No need to explicitly add Requires: glib2

* No URL tag.

* Summary ended with dot.

* %{_sysconfdir}/sysconfig/irqbalance should be marked ad config

That's all issues, I found so far in the spec-file, but there are some issues in the init-script as well:

[petro@Sulaco SPECS]$ rpmlint ../RPMS/ppc/irqbalance-*
irqbalance.ppc: W: obsolete-not-provided kernel-utils
irqbalance.ppc: E: malformed-line-in-lsb-comment-block # 
irqbalance.ppc: W: missing-lsb-keyword Required-Start in /etc/rc.d/init.d/irqbalance
irqbalance.ppc: W: missing-lsb-keyword Required-Stop in /etc/rc.d/init.d/irqbalance
irqbalance.ppc: W: service-default-enabled /etc/rc.d/init.d/irqbalance
irqbalance.ppc: W: service-default-enabled /etc/rc.d/init.d/irqbalance
2 packages and 0 specfiles checked; 1 errors, 5 warnings.
[petro@Sulaco SPECS]$ 

Actually, I'm not sure, whether this package should be started by default or not, but I'm sure other issues should be addressed.


Here is the spec-file with all my suggestions:

http://peter.fedorapeople.org/irqbalance.spec

Comment 7 Peter Lemenkov 2009-07-31 10:16:17 UTC
Fixed rpmlint error in the init-script and made some cosmetic changes in the spec-file:

http://peter.fedorapeople.org/irqbalance.spec
http://peter.fedorapeople.org/irqbalance-0.55-17.fc11.src.rpm

Comment 8 Peter Lemenkov 2009-07-31 18:05:39 UTC
Ok, I just uploaded my fixes and requested rebuild for devel branch.

I think this ticket can be closed now, and package is 

APPROVED.


Note You need to log in before you can comment on or make changes to this bug.