Spec URL: http://people.redhat.com/mitr/packaging/system-config-audit.spec SRPM URL: http://people.redhat.com/mitr/packaging/system-config-audit-0.4.11-1.src.rpm Description: An utility for editing audit configuration.
1) usermode. Ick. 2) This needs serious UI work, the spacing is all fubar. 3) Who exactly is the intended target of this? Our general admins don't want to use X based tools. Our desktop users don't care.
Just 3 issues I labeled with a letter in case discussion is needed. Looking over the spec file: A) release should be 1%{?dist} I get the following errors from rpmlint: B) system-config-audit.src: E: no-cleaning-of-buildroot %install C) system-config-audit.src: E: no-buildroot-tag
Thanks for the review. (In reply to comment #2) > Just 3 issues I labeled with a letter in case discussion is needed. Looking > over the spec file: > A) release should be 1%{?dist} Dist tag usage is not mandatory: https://fedoraproject.org/wiki/Packaging:DistTag#Do_I_Have_To_Use_the_Dist_Tag.3F > I get the following errors from rpmlint: > > B) system-config-audit.src: E: no-cleaning-of-buildroot %install > C) system-config-audit.src: E: no-buildroot-tag These are done automatically by recent RPM versions: https://fedoraproject.org/wiki/PackagingGuidelines#BuildRoot_tag https://fedoraproject.org/wiki/PackagingGuidelines#Prepping_BuildRoot_For_.25install
Good: + Basename of the SPEC file matches with package name + Package has a proper project homepage + Could downloading source via spectool -g * Consistently usage of rpm macros + Package tarball matches with upstream (md5sum: 2f47c6549b6adee06126243ec2162cb9) + Package contains no subpackages + Package has a valid licen tag + The license tag states GPLv2 as a valid OSS license + Copyrigh notes on headers matches with license tag + Package contains a verbatin copy of the license text + Local build works fine + Package support SMP build + Package use %find_lang macro + Rpmlint is silent on binary rpm. + Debunginfo package contains source files + Koji build works fine + Files has proper permisions + All packaged files are owned by the package + No packaged file belong to another package + %Ddoc is small, so we don't need a separate subpackage + Package contains proper %ChangeLog Bad: - Package contains not a BuildRoot tag. - Buildroot will not been clean on the start of %clean and %install - The package description could be more verbose. As a minimum you should wrote: this package provide a GUI which allows the user to configure the linux audit subsystem. - Please create a conflict statement agains the audit package before the splitt off.
Thanks for the review. (In reply to comment #4) > Bad: > - Package contains not a BuildRoot tag. https://fedoraproject.org/wiki/PackagingGuidelines#BuildRoot_tag says it is not necessary any more. > - Buildroot will not been clean on the start of %clean and %install Same here - https://fedoraproject.org/wiki/PackagingGuidelines#Prepping_BuildRoot_For_.25install for %install. %clean does clean the buildroot. > - The package description could be more verbose. > As a minimum you should wrote: > this package provide a GUI which allows the user to > configure the linux audit subsystem. Thanks, fixed. > - Please create a conflict statement agains the audit package > before the splitt off. AFAICS this should not be necessary - system-config-audit was a separate subpackage of audit. There are no file conflicts between the old audit package and any system-config-audit package, I can't see why the old audit package and a new system-config-audit package could not coexist. Updated package: http://people.redhat.com/mitr/packaging/system-config-audit.spec http://people.redhat.com/mitr/packaging/system-config-audit-0.4.11-2.src.rpm
What's the status on this bug? This is blocking release of audit-2.0. Thanks.
OK; I have take a short look on it and it seems to be ok, so I can approve your package.
New Package CVS Request ======================= Package Name: system-config-audit Short Description: Utility for editing audit configuration Owners: mitr Branches: InitialCC:
cvs done.