Bug 1087986 - Review Request: perl-Class-Forward - Namespace Dispatch and Resolution
Summary: Review Request: perl-Class-Forward - Namespace Dispatch and Resolution
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: David Dick
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-04-15 18:50 UTC by Emmanuel Seyman
Modified: 2014-04-20 14:33 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2014-04-20 14:33:46 UTC
Type: ---
Embargoed:
ddick: fedora-review+
petersen: fedora-cvs+


Attachments (Terms of Use)

Description Emmanuel Seyman 2014-04-15 18:50:43 UTC
Spec URL: http://people.parinux.org/~seyman/fedora/perl-Class-Forward/perl-Class-Forward.spec
SRPM URL: http://people.parinux.org/~seyman/fedora/perl-Class-Forward/perl-Class-Forward-0.100006-1.fc20.src.rpm
Description:
Class::Forward is designed to resolve Perl namespaces from shorthand (which
is simply a file-path-like specification). Class::Forward can also be used
to dispatch method calls using said shorthand. See the included exported
functions for examples on how this can be used.

Fedora Account System Username: eseyman

Comment 1 David Dick 2014-04-16 08:07:25 UTC
Licensing is fine.

FIX:

BR: perl(Exporter) # line 10 of lib/Class/Forward

TODO:

Change the by author Source0 to a by module URL

"Source0: http://www.cpan.org/modules/by-module/Class/Class-Forward-%{version}.tar.gz"

This will protect you if the author changes in the future.

You can remove the

"find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;"

line.  It is not required except for EL5.

You can update the 

"make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT"

to

"make pure_install DESTDIR=$RPM_BUILD_ROOT"

reference can be found at https://fedoraproject.org/wiki/Perl/Tips?rd=PackagingTips/Perl#Makefile.PL_vs_Build.PL

Comment 2 Ralf Corsepius 2014-04-16 08:31:17 UTC
(In reply to David Dick from comment #1)

> TODO:
> 
> Change the by author Source0 to a by module URL
There is no such rule in the FPG.

Conversely, it is commonly recommended practice to use by-author URLs and to not use by-module URLs, because by-module URLs have shown to be unreliable.

IIRC, there even once was a rule in some FPG-appendix, which mandated using by-author URLs, because of this, but I can't find it right now nor do I recall what has happened to this.

Comment 3 David Dick 2014-04-16 10:01:00 UTC
Hi Ralf,

If Emmanuel would prefer to stay with by-author, i have no problems with approving the package (hence the TODO section)

In my experience, i've had modules that i depend on change author (original authors hand over modules to new maintainers), causing me to rewrite spec files (non offical fedora/redhat) to keep up.

Comment 4 David Dick 2014-04-16 10:07:49 UTC
Hi Emmanuel,

I forgot an additional TODO:

Removal of the "cpanfile" file from %doc.

This file seems to be just specifying dependencies for the package.

Comment 5 Emmanuel Seyman 2014-04-16 21:16:53 UTC
(In reply to David Dick from comment #1)
> Licensing is fine.
> 
> FIX:
> 
> BR: perl(Exporter) # line 10 of lib/Class/Forward

Done.

(In reply to David Dick from comment #3)
>
> If Emmanuel would prefer to stay with by-author, i have no problems with
> approving the package (hence the TODO section)

The vast majority of the Perl packages I maintain have by-author Source0 lines
so I'll stick with that.

> In my experience, i've had modules that i depend on change author (original
> authors hand over modules to new maintainers), causing me to rewrite spec
> files (non offical fedora/redhat) to keep up.

Indeed, I've seen maintainers cease to maintain packages over stuff like this.

(In reply to David Dick from comment #4)
>
> Removal of the "cpanfile" file from %doc.
> 
> This file seems to be just specifying dependencies for the package.

Done.

Spec URL: http://people.parinux.org/~seyman/fedora/perl-Class-Forward/perl-Class-Forward.spec
SRPM URL: http://people.parinux.org/~seyman/fedora/perl-Class-Forward/perl-Class-Forward-0.100006-2.fc20.src.rpm

Comment 6 David Dick 2014-04-17 07:38:14 UTC
APPROVED

Comment 7 Emmanuel Seyman 2014-04-17 08:26:14 UTC
Thanks for the review, David.

New Package SCM Request
=======================
Package Name: perl-Class-Forward
Short Description: Namespace Dispatch and Resolution
Owners: eseyman
Branches: f20
InitialCC: perl-sig

Comment 8 Jens Petersen 2014-04-17 10:08:21 UTC
Git done (by process-git-requests).

Comment 9 Emmanuel Seyman 2014-04-20 14:33:46 UTC
Package is in Rawhide and F20. Thanks for the git repo, Jens.


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