Bug 476536
| Summary: | Review Request: zapplet - Zenoss monitoring tray applet | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | David Nalley <david> |
| Component: | Package Review | Assignee: | Brian Pepple <bdpepple> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, notting |
| Target Milestone: | --- | Flags: | bdpepple:
fedora-review+
kevin: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2009-02-17 13:30:16 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
David Nalley
2008-12-15 14:20:17 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'
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! 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. 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! 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. New Package CVS Request ======================= Package Name: zapplet Short Description: Zenoss Tray monitoring applet Owners: ke4qqq Branches: F-9 F-10 cvs done. zapplet-0.1-4.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/zapplet-0.1-4.fc9 zapplet-0.1-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/zapplet-0.1-4.fc10 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 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 If this has been built, and pushed to stable this bug can be closed. |