Bug 480552
Summary: | Review Request: poweradmin - A friendly web-based DNS administration tool for Bert Hubert's PowerDNS server | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ruben Kerkhof <ruben> |
Component: | Package Review | Assignee: | Kevin Fenzi <kevin> |
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: | kevin:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-01-26 13:19:53 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
Ruben Kerkhof
2009-01-18 21:09:14 UTC
I'd be happy to review this package. Look for a full review here in a bit. OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPLv3+) See below - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: cf83b89c6931160684de52a0b3ea3678 poweradmin-2.1.2.tgz cf83b89c6931160684de52a0b3ea3678 poweradmin-2.1.2.tgz.orig OK - BuildRequires correct - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. OK - No rpmlint output. OK - final provides and requires are sane SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should have dist tag OK - Should package latest version Issues: 1. Seems like all the files that refer to the license say "or later", so shouldn't this be 'GPLv3+' ? 2. You seem to have a typo in your Source0 line. A 'i' that shouldn't be there. 3. You should probibly require 'httpd' instead of 'webserver' as you are putting files in httpd specific locations. 4. The URL seems wrong... www.poweradmin.org instead of www.poweradmin.com ? Hi Kevin, thanks for the review. > 1. Seems like all the files that refer to the license say "or later", so > shouldn't this be 'GPLv3+' ? Fixed. > 2. You seem to have a typo in your Source0 line. A 'i' that shouldn't be there. Ah, fat fingers. Fixed as well. > 3. You should probibly require 'httpd' instead of 'webserver' as you are > putting files in httpd specific locations. Hmm, interesting issue. On the one hand, poweradmin works fine with lighttpd under fastcgi for example, on the other hand, most people will run this under apache. Adding the httpd conf file makes it run almost out of the box for the most users. But if you want to run poweradmin under another http server, it would still drag in httpd as a dependency. phpMyAdmin does exactly the same btw. What do you think? > 4. The URL seems wrong... www.poweradmin.org instead of www.poweradmin.com ? Oops, fixed as well. New spec: http://ruben.fedorapeople.org/poweradmin.spec New srpm: http://ruben.fedorapeople.org/poweradmin-2.1.2-2.fc11.src.rpm 1, 2, 4: looks good. 3. Yeah, there isn't a great solution here. I think for now at least you need to require httpd, as you are putting files in a directory it owns and you need to either own it or require it for the directory. Perhaps it's worth mailing the packaging list and see if someone can come up with a clever way around this problem? Change the requires to httpd, and this package is APPROVED. Thanks, I'll take it up with the packaging list. Until then I'll add a requires for httpd. New Package CVS Request ======================= Package Name: poweradmin Short Description: A friendly web-based DNS administration tool for Bert Hubert's PowerDNS server Owners: ruben Branches: F-9 F-10 EL-4 EL-5 cvs done. Package Change Request ====================== Package Name: poweradmin Owners: ruben slankes I accidentally retired the F-14 branch instead of releasing ownership. Can you please unretire this branch and change ownership to slankes if possible? I think I've processed things as requested; please double check. Yes, thanks a lot Jason! |