Bug 476536 - Review Request: zapplet - Zenoss monitoring tray applet
Review Request: zapplet - Zenoss monitoring tray applet
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Brian Pepple
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-15 09:20 EST by David Nalley
Modified: 2009-02-17 08:30 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-17 08:30:16 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
bdpepple: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description David Nalley 2008-12-15 09:20:17 EST
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 17:27:46 EST
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 01:18:26 EST
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 17:36:50 EST
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 17:57:18 EST
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-13 19:40:41 EST
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-14 22:14:59 EST
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 15:15:14 EST
cvs done.
Comment 8 Fedora Update System 2009-01-19 08:11:06 EST
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 08:11:10 EST
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 16:37:49 EST
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 16:38:14 EST
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 11:22:04 EST
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.