Bug 466906 - Review Request: perl-NOCpulse-SetID - Provides api for correctly changing user identity
Summary: Review Request: perl-NOCpulse-SetID - Provides api for correctly changing use...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dennis Gilmore
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: F-Spacewalk
TreeView+ depends on / blocked
 
Reported: 2008-10-14 13:18 UTC by Miroslav Suchý
Modified: 2009-02-09 11:17 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-09 11:17:14 UTC
Type: ---
Embargoed:
dennis: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Miroslav Suchý 2008-10-14 13:18:42 UTC
SPEC: http://miroslav.suchy.cz/fedora/perl-NOCpulse-SetID/perl-NOCpulse-SetID.spec
SRPM: http://miroslav.suchy.cz/fedora/perl-NOCpulse-SetID/perl-NOCpulse-SetID-1.5.5-1.f10.src.rpm
Description:
NOCpulse provides application, network, systems and transaction monitoring,
coupled with a comprehensive reporting system including availability,
historical and trending reports in an easy-to-use browser interface.

This package provides API for correctly changing user identity.


Scratch build in Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=880048

$ rpmlint perl-NOCpulse-SetID-1.5.5-1.f10.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint perl-NOCpulse-SetID-1.5.5-1.fc10.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 2 Dennis Gilmore 2008-11-03 14:36:46 UTC
builds fine in mock,  there is some issues though:

There is no way to verify the license.  The URL points at the generic spacewalk page and i could not find a page that talked about this package. at the least there should be a page for the tarball that has some contact info, a description and links to release tarballs.  There really should be a LICENSE file in the tarball and a header on the souce .pm file and the test .pl file  that state what the license is.

Comment 3 Miroslav Suchý 2008-11-03 16:28:52 UTC
Dennis com'on you are part of team which say under which license it is released.
But OK:

Updated SPEC
http://miroslav.suchy.cz/fedora/perl-NOCpulse-SetID/perl-NOCpulse-SetID.spec
Updated SRPM:
http://miroslav.suchy.cz/fedora/perl-NOCpulse-SetID/perl-NOCpulse-SetID-1.6.8-1.f10.src.rpm

There will be no page with description and links to released tarballs unless you will make it :)

Comment 4 Dennis Gilmore 2009-01-29 05:25:59 UTC
sorry for dropping the ball on this.

Builds in mock
source matches upstream.
rpmlint is silent.
sane Requires/Provides

approved.

Comment 5 Miroslav Suchý 2009-01-29 07:18:48 UTC
New Package CVS Request
=======================
Package Name: perl-NOCpulse-SetID
Short Description: Provides api for correctly changing user identity
Owners: msuchy
Branches: devel F-10 EL-4 EL-5
InitialCC:
Cvsextras Commits: yes

Comment 6 Ralf Corsepius 2009-01-29 08:03:41 UTC
(In reply to comment #4)
> sane Requires/Provides

Not quite :(

Requires(pre): perl(Class::MethodMaker)
This is very likely wrong.


Furthermore:
- Package is noarch => OPTIMIZE doesn't make any sense

- Please add perl-sig to InitialCC like most perl-packagers do.

- "make test" is claimed to require "root". 
I recommend to add an rpm option to run this testsuite 
(rpmbuild --with tests or similar). 
Better: Make the testsuite degrade gracefully if not being run as "root".

Comment 7 Miroslav Suchý 2009-01-29 10:04:34 UTC
> Requires(pre): perl(Class::MethodMaker)
> This is very likely wrong.

OK. There it seems that pure 
 Requires: perl(Class::MethodMaker)
Should be ok.

> Package is noarch => OPTIMIZE doesn't make any sense
OK. Will remove it.

> Please add perl-sig to InitialCC like most perl-packagers do.
I did not know it. I overlooked it in guidelines. Will do next time. 

> - "make test" is claimed to require "root". 
> I recommend to add an rpm option to run this testsuite
All the test in this package require root. So making them optional will be the same as skipping it entirely. IMHO.

Comment 8 Ralf Corsepius 2009-01-29 14:48:29 UTC
(In reply to comment #7)
> > Requires(pre): perl(Class::MethodMaker)
> > This is very likely wrong.
> 
> OK. There it seems that pure 
>  Requires: perl(Class::MethodMaker)
> Should be ok.
No, it's entirely superfluous, rpm adds it automatically.
You can completely remove it.

> > Please add perl-sig to InitialCC like most perl-packagers do.
> I did not know it. I overlooked it in guidelines. Will do next time. 
> 
> > - "make test" is claimed to require "root". 
> > I recommend to add an rpm option to run this testsuite
> All the test in this package require root.
Hmm, sounds like a pretty severe design problem with this package to me.

> So making them optional will be the
> same as skipping it entirely. IMHO.
Not at all. It would enable those who want to build rpms as "root" (likely you) to test your package.

Comment 9 Kevin Fenzi 2009-02-03 04:13:58 UTC
cvs done.


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