Bug 476536 - Review Request: zapplet - Zenoss monitoring tray applet
Summary: Review Request: zapplet - Zenoss monitoring tray applet
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Brian Pepple
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-12-15 14:20 UTC by David Nalley
Modified: 2009-02-17 13:30 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-17 13:30:16 UTC
Type: ---
Embargoed:
bdpepple: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description David Nalley 2008-12-15 14:20:17 UTC
Spec URL: http://ke4qqq.fedorapeople.org/zapplet.spec
SRPM URL: http://ke4qqq.fedorapeople.org/zapplet-0.1-1.src.rpm
Description: Zapplet is a tray applet for monitoring aspects of your Zenoss installation from the desktop. The intention is to work from all desktops supported by GTK.

I need a sponsor 


Here is the output from rpmlint:
[ke4qqq@nalleyt61 SPECS]$ rpmlint ./zapplet.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
[ke4qqq@nalleyt61 SPECS]$ rpmlint ../SRPMS/zapplet-0.1-1.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[ke4qqq@nalleyt61 SPECS]$ rpmlint ../RPMS/noarch/zapplet-0.1-1.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 1 Fabian Affolter 2008-12-27 22:27:46 UTC
Just some comments on your spec file

- The echo call in the %prep section is a bit unusual
- The license is not GPLv2, it's GPLv2+ according to the header in the source file
- One line per BR would be nice
- There is no need for '\' in the description
- From my point of view, there is no need for 'chmod 755 %{buildroot}%{_bindir}/%{name}'

The guidelines says that the BuildRequires for python packages should be 'BuildRequires: python' and the egg stuff 'BuildRequires: python-setuptools-devel'

Comment 2 David Nalley 2008-12-28 06:18:26 UTC
Fabian - I believe I have addressed all of your comments you can find the new spec and srpm files here: 

http://ke4qqq.fedorapeople.org/zapplet-0.1-2.src.rpm
http://ke4qqq.fedorapeople.org/zapplet.spec

Thanks for taking the time to review the package!

Comment 3 Brian Pepple 2008-12-28 22:36:50 UTC
Couple of quick items:
1. The requires on python is unnecessary. The requires: python(abi) = 2.6 in the build log does this for you.
2. Likewise the BR on python-setuptools is also unnecessary.  The -devel package will pull this in for you.
3. I haven't had a chance to look at the source closely yet, but I'm thinking the requires should be pygtk2, not gtk2.

Comment 4 David Nalley 2008-12-28 22:57:18 UTC
Brian: 

I believe I have addressed all of the above items, and you are correct about pygtk2.
 

New spec file: 
http://ke4qqq.fedorapeople.org/zapplet.spec
New SRPM: 
http://ke4qqq.fedorapeople.org/zapplet-0.1-3.src.rpm

[ke4qqq@nalleyt61 SPECS]$ rpmlint ../SRPMS/zapplet-0.1-3.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[ke4qqq@nalleyt61 SPECS]$ rpmlint ../RPMS/noarch/zapplet-0.1-3.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[ke4qqq@nalleyt61 SPECS]$ rpmlint ./zapplet.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings


Thanks for taking the time to review this package!

Comment 5 Brian Pepple 2009-01-14 00:40:41 UTC
Sorry about not being able to finish this up sooner, but I've been swamped with work the last few weeks.

Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Valid license tag.
* Buildroot has all required elements
* All paths begin with macros
* All necessary BuildRequires listed.
* Rpmlint produces no warnings or errors.
* Package builds in Mock fine.

Note: I was unable to test that this works since I don't have a Zenoss installation available.

+1 APPROVED.  Apply for the packager group in FAS, and I'll be your sponsor.

Comment 6 David Nalley 2009-01-15 03:14:59 UTC
New Package CVS Request
=======================
Package Name: zapplet
Short Description: Zenoss Tray monitoring applet
Owners: ke4qqq
Branches: F-9 F-10

Comment 7 Kevin Fenzi 2009-01-15 20:15:14 UTC
cvs done.

Comment 8 Fedora Update System 2009-01-19 13:11:06 UTC
zapplet-0.1-4.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/zapplet-0.1-4.fc9

Comment 9 Fedora Update System 2009-01-19 13:11:10 UTC
zapplet-0.1-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/zapplet-0.1-4.fc10

Comment 10 Fedora Update System 2009-01-21 21:37:49 UTC
zapplet-0.1-4.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update zapplet'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-0837

Comment 11 Fedora Update System 2009-01-21 21:38:14 UTC
zapplet-0.1-4.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing-newkey update zapplet'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-0850

Comment 12 Brian Pepple 2009-02-15 16:22:04 UTC
If this has been built, and pushed to stable this bug can be closed.


Note You need to log in before you can comment on or make changes to this bug.