I reviewed it as best I could and didn't find any problems.

It is GPL v2, rpmlint only throws one warning about using spaces instead of tabs
in the specfile, it meets packaging and naming standards as far as I can tell etc.

Interestingly, the tarball in this srpm and the one upstream are not the same;
several files differ, though all seem to differ by comments and whitespace.

A few rpmlint complaints:

  kerneloops.src: W: mixed-use-of-spaces-and-tabs (spaces: line 53,
   tab: line 1)
Not a big deal.

  kerneloops.x86_64: E: script-without-shebang
   /etc/dbus-1/system.d/kerneloops.dbus
I think this shouldn't be executable; the other files I see there don't seem
to be.

  kerneloops.x86_64: W: non-conffile-in-etc
   /etc/xdg/autostart/kerneloops-applet.desktop
This is OK.

  kerneloops.x86_64: W: spurious-executable-perm
   /usr/share/man/man8/kerneloops.1.gz
This shouldn't be executable either.

  kerneloops.x86_64: W: service-default-enabled /etc/rc.d/init.d/kerneloops
Services shouldn't be enabled by default.  This usually means the first entry 
on the chkconfig: line in the initscript should be '-'; I guess Default-Start:
should be either not present, empty, or '-' as well, but I'm not sure which it
should be, or if we even have anything that pays attention to it.

  kerneloops.x86_64: W: incoherent-subsys /etc/rc.d/init.d/kerneloops $prog
This is bogus.


Other issues:

The scriptlets to start and stop the service differ from the recommended ones.
For example, the %preun script won't trigger on package removals, so removing
the package will leave the service still running.  And it looks like a %postun
script was intended (given the dependency for it) but it's not actually in the
spec.  See http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

I've not seen a Makefile call desktop-file-itself, but don't see anything
wrong with the way it's being called so I don't see any reason for the spec to
call it explicitly.


Checklist:
X source files don't match upstream:
  2c5b6937983ea046d74359cb470e9002a329192998c7f4e50c1121ae6c381dc9
   kerneloops-0.9.tar.gz
  600aa09dbeaa439e1268f514b8b4bdcf0c51c2efbb26e23a1925e05387581b71  
   ../kerneloops-0.9.tar.gz

* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint has some valid complaints 
* final provides and requires are sane:
   config(kerneloops) = 0.9-1.fc9
   kerneloops = 0.9-1.fc9
  =
   /bin/bash
   /bin/sh
   chkconfig
   config(kerneloops) = 0.9-1.fc9
   initscripts
   libcurl.so.4()(64bit)
   libdbus-1.so.3()(64bit)
   libdbus-glib-1.so.2()(64bit)
   libglib-2.0.so.0()(64bit)
   libgobject-2.0.so.0()(64bit)
   libgtk-x11-2.0.so.0()(64bit)
   libnotify.so.1()(64bit)

* %check is present but disabled; according to comments, the test suite is
   broken upstream.  I have not attempted to test this package.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %find_lang used properly to collect translations.
X service management scriptlets are not the recommended ones.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Thanks for the quick review;

The file permission thing was a bug in the upstream package; I've fixed that.
I've copied the script templates from your URL into the spec
The tar mismatch was a timelag between when I built the rpms and when I ran 
the tar upload script; will not happen again (script will also have to run an 
rpmbuild ;)

The "service starts by default"... this service really needs to start by 
default, that's the whole point of it.

I've released version 0.10 of the program to fix the upstream package bugs;
http://www.kerneloops.org/download/  has the tar, src.rpm and binary rpm

Looks much better.  A new rpmlint warning has appeared:
  kerneloops.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/kerneloops.dbus
Looking at whether other packages have these files marked %config, it seems that
many packages do, although several like NetworkManager, ConsoleKit, cups and gdm
don't.  However, all package I have installed on my system actually call the
files *.conf.  We really don't have any guidelines here as far as I can tell so
honestly I'm not going to complain.

Everything now matches upstream:
   ea3bb4779ec74e2af5556d8551dbde6460c41930d61d0f5ef9924dfb42229deb  
   kerneloops-0.10.tar.gz

The permission issues are fixed.

About enabling the service by default, you could make the argument that the
whole point of httpd is to serve web pages, and yet it's not started by default.
 However, this service isn't network-facing and so I can't really make a
security argument.  And my personal preferences aside, we don't have any
specific guideline regarding the issue, so I won't block on it.

So, it seems that all the complaints I had are either addressed or moot.

APPROVED

What happens next? I don't see kerneloops in the PackageDB yet...

New Package CVS Request
=======================
Package Name: kerneloops
Short Description: Tool to automatically collect and submit kernel crash signatures
Owners: cebbert
Branches: F-8
InitialCC: 
Cvsextras Commits: no

cvs done. 

Any particular reason for the cvsextras commits: no? 

**** Access denied: cebbert is not in ACL for rpms/kerneloops/devel


Isn't the owner allowed to update the package?

(In reply to comment #9)
> **** Access denied: cebbert is not in ACL for rpms/kerneloops/devel
> 
> 
> Isn't the owner allowed to update the package?
> 

I added myself to the committers.

Package acls are generated twice an hour... It sounds like you tried a commit
after I added it, but before the :31 acl generating job ran. 

Try again and it should work fine? 

a job runs at the top of the hour that updates the acls in CVS  you need to 
wait for that to happen

Should fedora_requires_release_note be set now that the package has built?

Yep. Anytime you like. You might also add a note about what kind of info you
would like them to add for the release note. 

This should do for a release note:


This package contains the tools to collect kernel crash signatures,
and to submit them to the kerneloops.org website where the kernel
crash signatures get collected and grouped for presentation to the
Linux kernel developers.

The information will be used to debug kernel crashes and to see how
common they are, allowing kernel developers to concentrate on more
severe and common bugs.

Is there any reason this can't be closed now?

Was the release note done?

in reply to comment #17: 

Not yet I don't think... someone from docs should comment here and set the flag
to + (with a link to where it landed) or - (to say why they didn't think it was
release note worthy. ;( 

While we are waiting, "Any particular reason for the cvsextras commits: no?" 

My understanding is that the release note status is orthogonal to the resolution
of the ticket.  At least that's how we process the CVS queue; if the flag is '?'
we look at it, regardless of whether the ticket is open or now.

Honestly I'm just trying to get my bug list down, because it gets difficult to
track how many reviews I have in flight.

Closing this bug. This package already in Fedora and there is no need to pollute list of Review Requests 
with already reviewed package.