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=226410 Garrett Holmstrom <gholms@xxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |gholms@xxxxxxxxxxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |gholms@xxxxxxxxxxxxxxxxx Flag| |fedora-review? --- Comment #1 from Garrett Holmstrom <gholms@xxxxxxxxxxxxxxxxx> 2010-11-17 23:48:08 EST --- The most important issues with this package are outlined below. I will also attach a full review shortly. - License files installed when any subpackage combination is installed Unless the other packages pull in the docs subpackage you will need to put the "COPYING" file in another subpackage. (setroubleshoot-server, perhaps?) - Spec written in American English rpmlint nitpick: just change "gui" to "GUI" in the %description - Sources match upstream unless altered to fix permissibility issues Where are upstream's tarballs? - File permissions are sane /usr/lib64/python2.7/site-packages/setroubleshoot/default_encoding_utf8.so 0775 /usr/lib64/python2.7/site-packages/setroubleshoot/sesearch/_sesearch.so 0775 I assume these should actually be 0755? - GUI app installs .desktop file w/ desktop-file-install or has justification Please desktop-file-install instead of update-desktop-database or add a comment that justifies otherwise. - Scriptlets are sane What is the purpose of chowning %pkgdatabase in %post? - Changelog in prescribed format The entry for 3.0.6-1 seems to be mangled somehow. - Requires correct, justified where necessary Please depend on desktop-file-utils instead of /usr/bin/update-desktop-database for the %post and %postun scripts. Why do you Require libnotify when the generated packages already require libnotify.so.4? You can probably drop the fc7/8/11 conditionals now. - Config files marked with %config /etc/audisp/plugins.d/sedispatch.conf /etc/xdg/autostart/sealertauto.desktop I think the first needs to be %config(noreplace), while the second should get just %config. - %config files marked noreplace or justified /etc/dbus-1/system.d/org.fedoraproject.Setroubleshootd.conf /etc/dbus-1/system.d/org.fedoraproject.SetroubleshootFixit.conf /etc/setroubleshoot/setroubleshoot.cfg /usr/share/polkit-1/actions/org.fedoraproject.setroubleshootfixit.policy While it's understandable to have PolicyKit and dbus configs without noreplace, the spec file should contain commentary about it. - No %config files under /usr /usr/share/polkit-1/actions/org.fedoraproject.setroubleshootfixit.policy Unfortunately this is mandatory. If users aren't supposed to edit this then I would just remove the %config tag. I hope that helps! -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review