Red Hat Bugzilla – Bug 225789
Merge Review: genromfs
Last modified: 2015-05-04 21:32:55 EDT
Fedora Merge Review: genromfs http://cvs.fedora.redhat.com/viewcvs/devel/genromfs/ Initial Owner: pmachata@redhat.com
Tidied up version commited, not built. rpmlint is silent, for both source and binary rpm.
Appended %{?dist} to the Release tag and built as genromfs-0.5.2-4.fc13.
Clean fedora-review flag that the last reviewer left around.
Starting formal review.
The package is OK overall, I found just a couple nits: * rpmlint report: $ rpmlint x86_64/genromfs-0.5.2-4.fc13.x86_64.rpm \ genromfs-0.5.2-4.fc13.src.rpm genromfs.spec 2 packages and 1 specfiles checked; 0 errors, 0 warnings. * ExclusiveOS: I don't see a reason to state this explicitly. The fact that it's a Fedora package guarantees that it will only ever be compiled for Linux machines. * BuildRoot tag is ignored in Fedora 10+. Consider removing it. * Package COPYING, and consider also packaging NEWS (via %doc) * %files section should include %defattr(-,root,root,-) (Notice the last dash.)
(In reply to comment #5) Thanks for review! I've just committed the proposed changes to CVS, excluding the BuildRoot related one. (+ fixed spelling error with the latest rpmlint filesystem --> file system) > * ExclusiveOS: I don't see a reason to state this explicitly. The > fact that it's a Fedora package guarantees that it will only ever be > compiled for Linux machines. Good idea. 'cvs annotate' says the line has been there since the initial import. > * BuildRoot tag is ignored in Fedora 10+. Consider removing it. However removing it triggers a warning with the latest rpmlint: $ rpmlint --version rpmlint version 0.92 Copyright (C) 1999-2007 Frederic Lepied, Mandriva $ rpmlint genromfs.spec genromfs.spec: W: no-buildroot-tag 0 packages and 1 specfiles checked; 0 errors, 1 warnings. > * Package COPYING, and consider also packaging NEWS (via %doc) Fixed. > * %files section should include %defattr(-,root,root,-) (Notice the > last dash.) Fixed.
> > * BuildRoot tag is ignored in Fedora 10+. Consider removing it. > > However removing it triggers a warning with the latest rpmlint: Hmm, right. I think we could overrule rpmlint at this point, it's complaining about something that's in fact not mandatory per the guidelines. But I'll leave that up to you, I don't have any strong feelings either way. I think the package is now OK and I'm going to ACCEPT it. Thanks for cooperation.
The rest of the process doesn't really apply to merge review, so just close the review request as soon as you build that package with those fixes in.
Built as genromfs-0.5.2-5.fc13. Thanks!