Bug 192313
Summary: | Review Request: koan | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Michael DeHaan <mdehaan> |
Component: | Package Review | Assignee: | David Lutterkort <lutter> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | hbrock, jonathan.underwood |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-09-28 13:59:50 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: | |||
Bug Depends On: | 192311 | ||
Bug Blocks: | 163779 |
Description
Michael DeHaan
2006-05-18 21:11:29 UTC
A couple of quick comments: 1) The three macros at the top of the file are totally redundant. 2) Remove Vendor tag. 3) Relaease should contain the dist tag 4) The Prefix tag seems redundant ( I may be missing something there tho). 5) the install section should begin with rm -rf $RPM_BUILD_ROOT All rpmlint errors have been fixed and the packages have been updated at the above URLs. Please let me know if there's anything else that needs to be fixed. === The three macros at the top of the file are totally redundant. * Don't see this being a problem. Correct me if it breaks something. 2) Remove Vendor tag. * Fixed 3) Relaease should contain the dist tag * Fixed, per instructions on http://fedoraproject.org/wiki/DistTag ... please verify I got this right, though. As I understand it, my RPM's won't show any dist in the name until the build system tries to build them. 4) The Prefix tag seems redundant ( I may be missing something there tho). * Fixed (Removed) 5) the install section should begin with rm -rf $RPM_BUILD_ROOT * Fixed FYI ... This should have been in the original submission Cobbler (alluded to in the submission) has it's bugzilla here: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192311 Updated. - added doc URL - added directories - removed redundant macros - bumped release New RPM: http://et.redhat.com/~mdehaan/software/koan/koan-0.1.0-2.src.rpm New spec file: http://et.redhat.com/~mdehaan/software/koan/koan.spec Documentation URL as listed in spec: http://et.redhat.com/page/Cobbler_%26_Koan_Provisioning_Tools (largely a manpage copy for now) Those pages are 404'ing for me... Thanks for the heads up. These are the correct links: http://et.redhat.com/~mdehaan/software/cobbler/koan-0.1.0-2.src.rpm http://et.redhat.com/~mdehaan/software/cobbler/koan.spec Koan has been updated to take advantage of cobbler's new templating features. http://et.redhat.com/~mdehaan/software/cobbler/koan-0.1.1-3.noarch.rpm http://et.redhat.com/~mdehaan/software/cobbler/koan.spec Fixing a yaml import bug also present in cobbler: http://et.redhat.com/~mdehaan/software/cobbler/koan-0.1.1-4.src.rpm http://et.redhat.com/~mdehaan/software/cobbler/koan.spec Updating koan to match comments made on the cobbler submission (setup not quiet, odd permission on spec file) http://et.redhat.com/~mdehaan/software/cobbler/koan-0.1.1-5.src.rpm http://et.redhat.com/~mdehaan/software/cobbler/koan-0.1.1-5.noarch.rpm http://et.redhat.com/~mdehaan/software/cobbler/koan.spec rpmlint is clean on both rpms. Cobbler ( https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192311 ) was just approved (thanks jpmahowald) here, so this submission is no longer blocked waiting on cobbler approval. Seeing this programs are paired (client & server RPM's, essentially) it would be great if I could submit them both to FC-E at the same time. Two small comments: * Don't define the %name macro manually, setting 'Name:' will do that for you * The way you define INSTALLED_FILES leads to /usr/lib/python2.4/site-packages/koan/ and subdirs to not be owned by koan and sticking around after an uninstall. The %files section should read something like %files %defattr(-,root,root) %{_bindir}/koan /usr/lib/python?.?/site-packages/koan %{_mandir}/man1/koan.1.gz * Just as a suggestion that won't block this package: since you are upstream for this, you should include a copy of the GPL in COPYING, and %doc it in the specfile Newness: Source code of released builds is now available in Mercurial: http://hg.et.redhat.com/hg/emd/applications/koan New home for files: http://et.redhat.com/~mdehaan/software/repo/ Accessible via yum: http://et.redhat.com/~mdehaan/software/mdehaan.repo All of this info is updated on the Wiki page: http://et.redhat.com/page/Cobbler_%26_Koan_Provisioning_Tools In addition, the issues brought up by David L. above have been resolved. - %name macro removed - files section updated per packaging guidelines - COPYING file added Looks good; some more comments which won't hold up the review: - Include COPYING (and README. maybe AUTHORS) in the rpm with '%doc COPYING README' - The description has a small type (s/Koan standards/Koan stands/) - Remove the INSTALLED_FILES business from %install, since it's not used anymore I'll approve this as soon as you've been sponsored. Thanks: - %doc section fixed - typo fixed - INSTALLED_FILES fixed I've made the same fixes to cobbler. David: It doesn't look like this was ever changed to block FE-ACCEPT instead of FE-REVIEW. Could you please do so if you really approved this package? It helps with tracking to make sure all packages have been approved... Oops .. sorry; totally slipped through teh cracks. I did ACCEPT the package; changed blocker to FE-ACCEPT now |