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: nobody@fedoraproject.org ReportedBy: rohara@redhat.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com 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.
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 rohara@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
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 rohara@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841(FE-NEEDSPONSOR) | Depends on| |177841(FE-NEEDSPONSOR)
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 jkeating@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jkeating@redhat.com AssignedTo|nobody@fedoraproject.org |jkeating@redhat.com Flag| |fedora-review?
--- Comment #1 from Jesse Keating jkeating@redhat.com 2011-02-25 12:56:25 EST --- Taking this review (and sponsorship). Comments to follow soon.
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 jkeating@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(rohara@redhat.com | |)
--- Comment #2 from Jesse Keating jkeating@redhat.com 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.
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 rohara@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(rohara@redhat.com | |) |
--- Comment #3 from Ryan O'Hara rohara@redhat.com 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
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 jkeating@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #4 from Jesse Keating jkeating@redhat.com 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.
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 rohara@redhat.com 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
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 rohara@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
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 tibbs@math.uh.edu 2011-04-05 11:23:00 EDT --- Git done (by process-git-requests).
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 rohara@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Fixed In Version| |foghorn-0.1.2-4.fc15 Resolution| |NEXTRELEASE AssignedTo|jkeating@redhat.com |rohara@redhat.com Last Closed| |2011-04-19 11:09:24
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 rohara@redhat.com 2011-04-19 11:10:24 EDT --- Closing this as NEXTRELEASE. The foghorn pacakge is build for Fedora 15 and in testing stage.
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 kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR) Depends on|177841(FE-NEEDSPONSOR) |
package-review@lists.fedoraproject.org