Bug 187313 - (perl-Authen-Radius) Review Request: perl-Authen-Radius
Review Request: perl-Authen-Radius
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On: 187314
Blocks: FE-ACCEPT 187326
  Show dependency treegraph
 
Reported: 2006-03-29 17:25 EST by Andreas Thienemann
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-04-13 08:02:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Andreas Thienemann 2006-03-29 17:25:40 EST
Spec Name or Url: http://home.bawue.net/~ixs/perl-Authen-Radius/perl-Authen-Radius.spec
SRPM Name or Url: http://home.bawue.net/~ixs/perl-Authen-Radius/perl-Authen-Radius-0.12-1.src.rpm
Description:
This is RadiusPerl version 0.11. RadiusPerl is a Perl 5 module (Radius.pm)
which allows you to communicate with a Radius server from Perl. You can
just authenticate usernames/passwords via Radius, or comletely imitate
AAA requests and process server response.
Comment 1 Jason Tibbitts 2006-04-07 21:14:54 EDT
I'm not a Radius user but I can do a review.  The package builds fine in mock
(devel branch) and rpmlint is silent.

Issues:
BuildRequires: perl is not allowed; cpanspec should be able to generate a proper
specfile that doesn't do this.
There's no point in the BuildRequires: for perl(IO) and perl(Digest::MD5) as
these are part of base perl, but I don't supposed there's any harm in keeping
them.  The only thing that's strictly necessary is
BuildRequires: perl(Data::HexDump)

Review:
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written, uses macros consistently and
conforms to the Perl template.
* license field matches the actual license.
* license is open source-compatible.  It's not included separately in the
package, but this is not necessary as the upstream tarball does not include it.
* source files match upstream:
   8427f931a4e0e689ecf8ddf30e5a0ea7  RadiusPerl-0.12.tar.gz
   8427f931a4e0e689ecf8ddf30e5a0ea7  RadiusPerl-0.12.tar.gz-srpm
* package builds in mock.
X BuildRequires: perl not permitted.
* no shared libraries.
* package is not relocatable.
* owns the directory it creates.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.
* The package owns %{perl_vendorlib}/Authen, which will probably also be owned
by any module under the Authen:: namespace.  However, there are no dependencies
which could create this directory so there is no alternative but for this
package to own it.
Comment 2 Andreas Thienemann 2006-04-12 23:11:38 EDT
(In reply to comment #1)

> BuildRequires: perl is not allowed; cpanspec should be able to generate a proper
> specfile that doesn't do this.
Removed.

> There's no point in the BuildRequires: for perl(IO) and perl(Digest::MD5) as
> these are part of base perl, but I don't supposed there's any harm in keeping
> them.
Removed.

Thx for the review.
Upgraded files are available at: 
http://home.bawue.de/~ixs/perl-Authen-Radius/
Comment 3 Jason Tibbitts 2006-04-12 23:49:04 EDT
Looks good and builds properly.  BTW, you left the perl(IO) and
perl(Digest::MD5) buildreqs in, but this isn't a blocker.

APPROVED

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