Bug 480552 - Review Request: poweradmin - A friendly web-based DNS administration tool for Bert Hubert's PowerDNS server
Summary: Review Request: poweradmin - A friendly web-based DNS administration tool for...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-01-18 21:09 UTC by Ruben Kerkhof
Modified: 2011-03-09 16:08 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-01-26 13:19:53 UTC
Type: ---
Embargoed:
kevin: fedora-review+


Attachments (Terms of Use)

Description Ruben Kerkhof 2009-01-18 21:09:14 UTC
Spec URL: http://ruben.fedorapeople.org/poweradmin.spec
SRPM URL: http://ruben.fedorapeople.org/poweradmin-2.1.2-1.fc11.src.rpm
Description:
Poweradmin is a friendly web-based DNS administration tool for Bert Hubert's
PowerDNS server. The interface has full support for most of the features of
PowerDNS. It has full support for all zone types (master, native and slave),
for supermasters for automatic provisioning of slave zones, full support
for IPv6 and comes with multi-language support.

Comment 1 Kevin Fenzi 2009-01-18 23:02:05 UTC
I'd be happy to review this package. Look for a full review here in a bit.

Comment 2 Kevin Fenzi 2009-01-18 23:33:35 UTC
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 ?

Comment 3 Ruben Kerkhof 2009-01-19 12:40:02 UTC
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

Comment 4 Kevin Fenzi 2009-01-21 04:59:37 UTC
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.

Comment 5 Ruben Kerkhof 2009-01-22 12:13:03 UTC
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

Comment 6 Kevin Fenzi 2009-01-23 23:25:47 UTC
cvs done.

Comment 7 Ruben Kerkhof 2011-03-08 22:02:56 UTC
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?

Comment 8 Jason Tibbitts 2011-03-09 01:27:35 UTC
I think I've processed things as requested; please double check.

Comment 9 Ruben Kerkhof 2011-03-09 16:08:34 UTC
Yes, thanks a lot Jason!


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