[Bug 676392] New: Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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

Summary: Review Request: foghorn - DBus signal to SNMP trap service

https://bugzilla.redhat.com/show_bug.cgi?id=676392

Summary: Review Request: foghorn - DBus signal to SNMP trap
                    service
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora

Spec URL: http://people.redhat.com/~rohara/foghorn/foghorn.spec
SRPM URL: http://people.redhat.com/~rohara/foghorn/foghorn-0.1.2-1.el6.src.rpm

Description: The foghorn service listens for specific dbus signals and maps
those signals to SNMPv2 traps. This service is currently used by various
cluster components (fenced, corosync, rgmanager) as a means to generate SNMPv2
traps for certains events.

This is my first Fedora package and I will need a sponsor.

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Ryan O'Hara  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841(FE-NEEDSPONSOR)

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Ryan O'Hara  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|177841(FE-NEEDSPONSOR)      |
         Depends on|                            |177841(FE-NEEDSPONSOR)

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Jesse Keating  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #1 from Jesse Keating  2011-02-25 12:56:25 EST ---
Taking this review (and sponsorship).  Comments to follow soon.

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Jesse Keating  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |needinfo?([e-mail]
                   |                            |)

--- Comment #2 from Jesse Keating  2011-02-25 13:29:51 EST ---
Working through the spec I have come comments:

The summary mentions the package name, that's redundant info.

In the requires section you're manually adding requires for some things that
get picked up by library level autorequires.  This is duplicate data that just
bloats the rpm databases and should be removed.  Manual Requires lines should
only be there for things that the rpmbuild process doesn't automatically
discover.

Why do you have an ExclusiveArch setup?  There is no comment in the spec to
explain it.

The init script is not properly "installed" by using chkconfig, please see
https://fedoraproject.org/wiki/Packaging:SysVInitScript for the current
guidelines.

On to the rest of the review...

rpmlint has this to say:

foghorn.x86_64: W: unexpanded-macro dependency glib2 >= %{glib_version}
%{glib_version}
  I noticed this in the requires, looks like your macro use isn't correct here.
 glib vs glib2

foghorn.x86_64: W: name-repeated-in-summary C Foghorn
  I mentioned this above.

foghorn.x86_64: E: description-line-too-long C Foghorn listens for specific
DBUS signals and translates those signals to SNMP traps.
  Keep these lines trimmed to 80 chars or less.

foghorn.x86_64: E: zero-length /usr/share/doc/foghorn-0.1.2/README
  Don't include it if it's empty, or put something in it upstream.

foghorn.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/dbus-foghorn.conf
  I believe this file needs to be marked as a config file.

foghorn.x86_64: E: zero-length /usr/share/doc/foghorn-0.1.2/NEWS
  Same thing I wrote about the README file applies here.

foghorn.x86_64: E: init-script-without-chkconfig-postin
/etc/rc.d/init.d/foghorn
foghorn.x86_64: E: init-script-without-chkconfig-preun /etc/rc.d/init.d/foghorn
  I mentioned this above about the init script.

foghorn.x86_64: W: incoherent-subsys /etc/rc.d/init.d/foghorn $prog
  You can ignore this, because of the use of the variable "$prog"

foghorn.spec:29: W: mixed-use-of-spaces-and-tabs (spaces: line 29, tab: line 1)
  Please don't mix tabs and spaces :)

I looked over the licensing and looked at the source files, that all looks OK
Spec name is OK
Upstream tarball matches the tarball in the srpm

Everything else looks fine, please correct the issues listed above.

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Ryan O'Hara  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|needinfo?([e-mail] |
                   |)                           |

--- Comment #3 from Ryan O'Hara  2011-03-14 14:14:57 EDT ---
I've updated the spec file and successfully built a scratch rpm in koji. I
believe all the issues mentioned above have been addressed. Running the spec
file through rpmlint reports no errors/warnings.

http://people.redhat.com/~rohara/foghorn/foghorn.spec
http://people.redhat.com/~rohara/foghorn/foghorn-0.1.2-4.fc13.src.rpm

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Jesse Keating  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #4 from Jesse Keating  2011-03-21 13:27:36 EDT ---
Looks good.  I would remove the period after the summary though, just a warning
from rpmlint.

I'm approving.

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

--- Comment #5 from Ryan O'Hara  2011-04-04 15:25:45 EDT ---
New Package SCM Request
=======================
Package Name: foghorn
Short Description: D-Bus to SNMP service
Owners: rohara
Branches: f15
InitialCC: rohara

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Ryan O'Hara  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

--- Comment #6 from Jason Tibbitts  2011-04-05 11:23:00 EDT ---
Git done (by process-git-requests).

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Ryan O'Hara  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
   Fixed In Version|                            |foghorn-0.1.2-4.fc15
         Resolution|                            |NEXTRELEASE
         AssignedTo|[e-mail]         |[e-mail]
        Last Closed|                            |2011-04-19 11:09:24

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

--- Comment #7 from Ryan O'Hara  2011-04-19 11:10:24 EDT ---
Closing this as NEXTRELEASE. The foghorn pacakge is build for Fedora 15 and in
testing stage.

[Bug 676392] Review Request: foghorn - DBus signal to SNMP trap service

[Red Hat Bugzilla]

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=676392

Kevin Kofler  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841(FE-NEEDSPONSOR)
         Depends on|177841(FE-NEEDSPONSOR)      |