Bug 226410

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!

Created attachment 461207 [details]
Review for devel package setroubleshoot-3.0.8-1.fc15

Thanks this was great.

I have a couple of questions.

- Spec written in American English
What does that mean?

Upstream is a git repository.

http://git.fedorahosted.org/git/?p=setroubleshoot.git

Not sure how that is supposed to go into the spec file.

All other fixes should be in setroubleshoot-3.0.9

(In reply to comment #3)
> - Spec written in American English

That's just the heading I put rpmlint's capitalization complaint under.  If you've fixed that then don't worry about it.

> Upstream is a git repository.
> 
> http://git.fedorahosted.org/git/?p=setroubleshoot.git
> 
> Not sure how that is supposed to go into the spec file.

What you should do in that case is put comments above the Source0 line that tell people how to build tarballs identical to yours.  I would use these, personally:

# git clone git://git.fedorahosted.org/git/setroubleshoot.git; cd setroubleshoot
# git archive --prefix setroubleshoot-3.0.9/ 46de4d7b86bebb8375f69e78a88fe154c9a97800 > setroubleshoot-3.0.9.tar.gz

Then all you have to do is update the second line with the right info when you build a new release tarball.

Fixed in setroubleshoot-3.0.10-1.fc15