Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=508922 David Timms <dtimms@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dtimms@xxxxxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |dtimms@xxxxxxxxxxxx --- Comment #13 from David Timms <dtimms@xxxxxxxxxxxx> 2009-09-27 09:16:36 EDT --- (In reply to comment #12) > Can we get this approved, I really want to get it out in Fedora 12. First step is reviewed / improved ... (In reply to comment #11) I don't think I have enough experience to be approving security related packages in Fedora. However, some initial nits: 1. package name differs from web page, so maybe fix the web page from (System-Config-Selinx) ! 2. repeated requires: Requires: selinux-policy Requires: selinux-policy >= 3.6.28-4 -> which is it ?, I don't think it makes sense to have both. 3. Can you confirm that this package replaces policycoreutils-gui ? Does the new package include all the old package's functionality ? 4. The normal place to put the python_sitelib call is at the top of the .spec, see: http://fedoraproject.org/wiki/Packaging:Python#System_Architecture 5. description: typo in (graphcial). Perhaps instead of just repeating short names/acronyms, we could have something like spelling out the acronym during first stating of it: --- This package contains two graphical tools for adjusting the mandatory access control security settings, as implemented by Security Enhanced Linux (SELinux). system-config-selinux provides an interface for configuring and managing SELinux, while selinux-polgengui is used to generate SELinux policy modules. --- -> feel free to improve upon that ! 6. For consistency, keep the two-line breaks between sections (for post, clean files, install) 7. Nice to see lines like ln... to be split over two lines, limiting spec to be mostly <= 80 chars wide. 8. avoid use of tab char, space is easier to peruse. 9. changelog: is not in required format: - space between * and first char of date. - space between - and item text - version-release is not included for any entries. (think the space is missing as well, making those entries look weird. - subprocesss ! - lines longer than 80chars. 10. how were the labelling problems fixed (were they added to the -targeted policy) ? 11. Is there any doc files that need to be included ? 12. Who is going to own this package going forward, since it seems the reporter is having trouble keeping the spec file consistent with itself, let alone the fedora packaging standards, is this a time problem ? 13. files: -config(noreplace) %{_datadir}/ -> are people expected to make changes to configuration in /usr/share ? -%{_sysconfdir}/dbus-1/system.d/org.fedoraproject.selinux.config.conf -> is this a file that needs the noreplace, ie are users expected to adjust this (potentially) ? 14. gui tools require icons, are some included, do some need to be requested of the artwork project ? I haven't explicitly checked Provides nor BuildRequires. Hopefully this helps move the review a little further on. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review