[Bug 226410] Merge Review: setroubleshoot

bugzilla at redhat.com bugzilla at redhat.com
Thu Nov 18 04:48:10 UTC 2010

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


Garrett Holmstrom <gholms at fedoraproject.org> changed:

           What    |Removed                     |Added
             Status|NEW                         |ASSIGNED
                 CC|                            |gholms at fedoraproject.org
         AssignedTo|nobody at fedoraproject.org    |gholms at fedoraproject.org
               Flag|                            |fedora-review?

--- Comment #1 from Garrett Holmstrom <gholms at fedoraproject.org> 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/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

You can probably drop the fc7/8/11 conditionals now.

- Config files marked with %config

I think the first needs to be %config(noreplace), while the second should get
just %config.

- %config files marked noreplace or justified

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

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.

More information about the package-review mailing list