https://bugzilla.redhat.com/show_bug.cgi?id=887821 --- Comment #5 from Trond H. Amundsen <t.h.amundsen@xxxxxxxxxxx> --- (In reply to Johan Swensson from comment #4) > [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the > beginning of %install. > [!]: Buildroot is not present > [!]: Package has no %clean section with rm -rf %{buildroot} (or > $RPM_BUILD_ROOT) > > I don't think these should be considered blockers in this case > due to the fact that you're building for EL5 as well. Yes, these are required for EL5, unfortunately. > [!]: Each %files section contains %defattr if rpm < 4.4 > %defattr is not needed anymore, you may drop it during import into git I didn't know that, thanks for the tip. > Ideally the package should have been BuildArch: noarch > But that's not possible as discussed in your other > package review https://bugzilla.redhat.com/show_bug.cgi?id=743615 > You should mention why it's not noarch as a comment in the spec file. > Right above this comment would be a good place for such a comment. > # No binaries here, do not build a debuginfo package > %global debug_package %{nil} You're right, I should include that in a comment. Noted. > [!]: Uses parallel make %{?_smp_mflags} macro. > Make should always be invoked with %{?_smp_mflags} but in this case > as there isn't much to be built it should be fine. The %{?_smp_mflags} won't hurt, either. I'll include it. > [!]: Requires correct, justified where necessary. > You could Require nagios-common, as that is the owner of > %_libdir/nagios/plugins/ > But as nagios-plugins Requires nagios-common this is not currently a blocker. I guess things have changed slightly since I checked. I checked again to be sure, and it turns out that %_libdir/nagios/plugins/ is provided by nagios-common on EL6 and Fedora, and by nagios-plugins on EL5. I'll fix that with a conditional in the spec file. The package shouldn't require nagios-plugins if it isn't strictly necessary. > APPROVED Thank you so much for the review, and for your helpful comments! :) -trond -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review