Red Hat Bugzilla – Bug 225882
Merge Review: hdparm
Last modified: 2007-11-30 17:11:55 EST
Fedora Merge Review: hdparm
Initial Owner: firstname.lastname@example.org
I'm reviewing this one (not sponsored yet)
This one has several rpmlint errors:
rpmlint on ./hdparm-6.9-1.src.rpm
W: hdparm summary-ended-with-dot A utility for displaying and/or setting hard
- Remove the dot
E: hdparm tag-not-utf8 %changelog
E: hdparm non-utf8-spec-file hdparm.spec
- make sure that umlauted characters and such are utf8-encoded, not latin-1
E: hdparm no-cleaning-of-buildroot %install
- clean $RPM_BUILD_ROOT at the beginning of %install.
W: hdparm mixed-use-of-spaces-and-tabs (spaces: line 7, tab: line 9)
W: hdparm patch-not-applied Patch2: hdparm-6.3-idestruct.patch
- Remove the patch if it's no longer needed
* LICENSE.TXT must be included as a %doc
Other things look fine.
* md5sum matches upstream
* specfile is clean
* license is acceptable
* buildrequires is fine
* no locales, no shlibs, no relocations, etc.
should be fixed in hdparm-6.9-2, thanks for the review !
I'll leave this report open as suggested in
In reply to comment #3:
Yeah, since that was a review from a not yes sponsored reviewer, it should be
left open for an official review.
Thanks for looking at this Andy!
- BuildRoot is not the preferred value
- %build does not use smp flags; if not supported, please add a comment in the spec
- please also consider including README.accoustic; the feature is documented in
the man page, but the README seems to include a bit more info
The rest seems OK. Please fix the above and I'll do a full review.
ok, fixed in hdparm-6_9-3. I've added smp flags although it doesn't really
matter with just 2 .c files and a built time of <3 secs
I know and I agree with you. It's a matter of consistency among packages... and
maybe even insurance for the future.
I'll review after the servers sync.
- package meets naming guidelines
- package meets packaging guidelines
- license (BSD) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all files/directories that it creates, does not take ownership of foreign
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- just a small binary, no static, .la, .pc etc
- no scriptlets
- builds in mock
- runs as advertised
The only caveat I see is that, despite being passed RPM_BUILD_OPT in %build, it
also takes looooots of compile parameters from the Makefile. All seem sane
however, and are not very important since it's not an application from which to
squeeze each ounce of performance.