Bug 617400 - Review Request: pfHandle - wrapper for tools to manage postfix mail queue
Summary: Review Request: pfHandle - wrapper for tools to manage postfix mail queue
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-07-22 22:32 UTC by Carl Thompson
Modified: 2011-03-19 18:21 UTC (History)
2 users (show)

Fixed In Version: pfHandle-1.1-4.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-08-13 21:08:36 UTC
Type: ---
Embargoed:
kevin: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Carl Thompson 2010-07-22 22:32:55 UTC
Spec URL: http://www.red-dragon.com/pfHandle.spec
SRPM URL: http://www.red-dragon.com/pfHandle-1-1.src.rpm
Description: wrapper for tools to manage postfix mail queue

Second package submitted, first was https://bugzilla.redhat.com/show_bug.cgi?id=615669

Need a sponsor

This package and spec passed rpmlint with no errors
This package built on koji for fedora 13 and rawhide with no errors.

Comment 1 Kevin Fenzi 2010-07-24 19:37:32 UTC
I'll go ahead and review this and your other submission and look at sponsoring you. ;) 

Look for a full review in a while here...

Comment 2 Kevin Fenzi 2010-07-24 20:08:39 UTC
Some general comments before doing a full review here: 

- You probibly want a dist tag. If you don't have one it becomes difficult to ship the same version to multiple release branches. 

- You should get rid of the "[ "$RPM_BUILD_ROOT" != "/" ] && " constructs. 

- You should get rid of the Vendor tag

Fix those up and I can do the full review. ;)

Comment 3 Carl Thompson 2010-07-26 18:15:01 UTC
New
Spec URL: http://www.red-dragon.com/pfHandle.spec
SRPM URL: http://www.red-dragon.com/pfHandle-1-1.fc13.src.rpm

Comment 4 Kevin Fenzi 2010-07-28 03:59:12 UTC
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
See below - Meets Packaging Guidelines.
See below - License
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
52a127b0616657d05edc0cb21e91d172  pfHandle-1.tar.gz
52a127b0616657d05edc0cb21e91d172  pfHandle-1.tar.gz.orig

OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
See below - 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 - Package obey's FHS standard (except for 2 exceptions)
OK - No rpmlint output. 
 - 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
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues:

1. The only indicator of what the package license is is the launchpad site. ;)
Might be good to include a COPYING file and explicitly state in the README that it's
GPL v2 only.

2. You should nuke the
[ "$RPM_BUILD_ROOT" != "/" ] &&
from the clean section.

3. rpmlint says:
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 5 Carl Thompson 2010-07-28 04:39:01 UTC
SPEC: http://red-dragon.com/pfHandle.spec
SRPM: http://red-dragon.com/pfHandle-1-2.fc13.src.rpm

1) Added COPYING file and updated README

2) removed RPM_BUILD_ROOT from install and clean

3) rpmlint says
[redragon@gkar SPECS]$ rpmlint pfHandle.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
[redragon@gkar SRPMS]$ rpmlint pfHandle-1-2.fc13.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

koji build on f13 passed no failures
koji build on rawhide passed no failures

Comment 6 Kevin Fenzi 2010-07-28 17:10:36 UTC
Cool. We are getting there. ;) 

1. Thats fine, but: 

a) You can just do "%doc COPYING README". No need to use _defaultdocdir or the like. 
b) You seem to have changed the upstream release contents without changing the release. This is not good practice. Anytime you change the contents, make a new release. Now when people look at the '1' version they won't know if it's the one without COPYING and README changes or not. ;) 
c) You have in README:
"See COPYING for licensing information" Which is not explicit enough. Just based on copying, the license here would be "GPL+" ie, any version of the gpl ever published. ;) If you just want v2, say "This package is released under the GPLv2 only"

2. Looks good. 

3. Excellent.

Comment 7 Carl Thompson 2010-07-28 18:23:55 UTC
SPEC: http://red-dragon.com/pfHandle.spec
SRPM: http://red-dragon.com/pfHandle-1.1-3.fc13.src.rpm

cleaned up %doc section

Properly updated the upstream versioning

updated README to state its GPLv2 and that
the full license is in COPYING.

Comment 8 Kevin Fenzi 2010-07-28 19:22:47 UTC
ok, looks good. I see no further blockers now, so this package is APPROVED.

Comment 9 Carl Thompson 2010-07-28 19:49:48 UTC
New Package CVS Request
=======================
Package Name: pfHandle
Short Description: wrapper for postfix queue management tools
Owners: redragon
Branches: F-13
InitialCC: redragon

Comment 10 Kevin Fenzi 2010-07-30 20:33:09 UTC
GIT done (by process-cvs-requests.py).

with f14 branch added.

Comment 11 Fedora Update System 2010-07-30 23:48:29 UTC
pfHandle-1.1-3.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/pfHandle-1.1-3.fc13

Comment 12 Fedora Update System 2010-07-30 23:48:34 UTC
pfHandle-1.1-3.fc14 has been submitted as an update for Fedora 14.
http://admin.fedoraproject.org/updates/pfHandle-1.1-3.fc14

Comment 13 Fedora Update System 2010-08-01 19:22:59 UTC
pfHandle-1.1-3.fc14 has been pushed to the Fedora 14 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 pfHandle'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/pfHandle-1.1-3.fc14

Comment 14 Fedora Update System 2010-08-03 00:37:00 UTC
pfHandle-1.1-3.fc13 has been pushed to the Fedora 13 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 pfHandle'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/pfHandle-1.1-3.fc13

Comment 15 Fedora Update System 2010-08-13 21:08:31 UTC
pfHandle-1.1-3.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2010-08-19 01:12:50 UTC
pfHandle-1.1-3.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Carl Thompson 2011-03-01 23:13:00 UTC
Package Change Request
======================
Package Name: pfHandle
New Branches: EPEL5 EPEL6
Owners: redragon
InitialCC: redragon

Comment 18 Jason Tibbitts 2011-03-02 02:10:00 UTC
The branch names are EL-5 and EL-6; I fixed the request for you.

Git done (by process-git-requests).

Comment 19 Fedora Update System 2011-03-02 02:42:49 UTC
Package pfHandle-1.1-4.el5:
* should fix your issue,
* was pushed to the Fedora EPEL 5 updates-testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing pfHandle-1.1-4.el5'
as soon as you are able to, then reboot.
Please go to the following url:
https://admin.fedoraproject.org/updates/pfHandle-1.1-4.el5
then log in and leave karma (feedback).

Comment 20 Fedora Update System 2011-03-02 02:43:47 UTC
Package pfHandle-1.1-4.el6:
* should fix your issue,
* was pushed to the Fedora EPEL 6 updates-testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing pfHandle-1.1-4.el6'
as soon as you are able to, then reboot.
Please go to the following url:
https://admin.fedoraproject.org/updates/pfHandle-1.1-4.el6
then log in and leave karma (feedback).

Comment 21 Fedora Update System 2011-03-19 18:20:58 UTC
pfHandle-1.1-4.el5 has been pushed to the Fedora EPEL 5 stable repository.

Comment 22 Fedora Update System 2011-03-19 18:21:07 UTC
pfHandle-1.1-4.el6 has been pushed to the Fedora EPEL 6 stable repository.


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