Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 232165 (pear-File-Passwd) - Review Request: php-pear-File-Passwd - Manipulate many kinds of password files
Summary: Review Request: php-pear-File-Passwd - Manipulate many kinds of password files
Keywords:
Status: CLOSED NEXTRELEASE
Alias: pear-File-Passwd
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-14 01:50 UTC by Christopher Stone
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-05-15 04:39:04 UTC
Type: ---
Embargoed:
j: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Christopher Stone 2007-03-14 01:50:51 UTC
Spec URL: http://tkmame.retrogames.com/fedora-extras/php-pear-File-Passwd.spec
SRPM URL: http://tkmame.retrogames.com/fedora-extras/php-pear-File-Passwd-1.1.6-1.src.rpm

Description:
Provides methods to manipulate and authenticate against standard Unix,
SMB server, AuthUser (.htpasswd), AuthDigest (.htdigest), CVS pserver
and custom formatted password files.

Comment 1 Jason Tibbitts 2007-05-06 17:21:55 UTC
Builds fine and rpmlint is clean.  Everything looks good, although I'm curious
about the split of the -samba subpackage.  The additional dependencies from
php-pear-Crypt-CHAP only total about 200K, which isn't a really big deal but I
guess it is nonzero.  It looks like the code handles the missing Smb module well
(raising an exception instead of just bombing) so I don't suppose it will hurt
anything.  But if you were trying to save folks from having to install samba,
then there's not much point because even now the -samba module doesn't require
samba (just a few encryption libs).

You might also consider calling the subpackage "-Smb" since that's what the
module itself is called.  We don't really have any guidelines for this, so I
suppose it's up to you.

I don't see any blockers, though.

Review:
* source files match upstream:
   b01af05eb1c9714737ea5aba0b409c4dc6e6e62363a36a6c4b0a0d0048b69edb  
   File_Passwd-1.1.6.tgz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* rpmlint is silent.
* final provides and requires are sane:
  php-pear-File-Passwd-1.1.6-1.fc7.noarch.rpm
   php-pear(File_Passwd) = 1.1.6
   php-pear-File-Passwd = 1.1.6-1.fc7
  =
   /bin/sh
   /usr/bin/pear
   php-pear(PEAR)

  php-pear-File-Passwd-samba-1.1.6-1.fc7.noarch.rpm
   php-pear-File-Passwd-samba = 1.1.6-1.fc7
  =
   php-pear(Crypt_CHAP) >= 1.0.0
   php-pear-File-Passwd = 1.1.6-1.fc7

* %check is not present; no test suite upstream.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
   /usr/share/pear/File is also owned by php-pear-File but it is not a
   dependency (and it doesn't need to be).
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

APPROVED, I'll leave it to you to decide whether you really need the subpackage.

Comment 2 Christopher Stone 2007-05-13 15:36:26 UTC
Spec URL: http://tkmame.retrogames.com/fedora-extras/php-pear-File-Passwd.spec
SRPM URL:
http://tkmame.retrogames.com/fedora-extras/php-pear-File-Passwd-1.1.6-2.src.rpm

%changelog
* Sun May 13 2007 Christopher Stone <chris.stone> 1.1.6-2
- Include samba extension as default for Fedora
- Exclude samba extension from Enterprise Linux


Comment 3 Christopher Stone 2007-05-13 15:39:58 UTC
Can you please check this updated spec file, I'm not sure if I did this right:

%if !0%{?rhel}
...etc

Comment 4 Christopher Stone 2007-05-13 16:39:11 UTC
Nevermind, I just got thl's email and did an EL-5 mock build, everything looks
okay.  Thanks for the review!

New Package CVS Request
=======================
Package Name: php-pear-File-Passwd
Short Description: Manipulate many kinds of password files
Owners: chris.stone
Branches: EL-5 FC-5 FC-6
InitialCC: 



Comment 5 Christopher Stone 2007-05-15 04:39:04 UTC
Build on all branches succeeded, thanks for the review!


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