Bug 1079615

Summary:	Review Request: perl-MooX-HandlesVia - NativeTrait-like behavior for Moo
Product:	[Fedora] Fedora	Reporter:	Ralf Corsepius <rc040203>
Component:	Package Review	Assignee:	Petr Pisar <ppisar>
Status:	CLOSED ERRATA	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	package-review, ppisar
Target Milestone:	---	Flags:	ppisar: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:	perl-MooX-HandlesVia-0.001005-2.fc19	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2014-03-29 03:53:49 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	1079613
Bug Blocks:	1079473, 1079618

Description Ralf Corsepius 2014-03-22 04:54:30 UTC

Spec URL: http://corsepiu.fedorapeople.org/packages/perl-MooX-HandlesVia.spec
SRPM URL: http://corsepiu.fedorapeople.org/packages/perl-MooX-HandlesVia-0.001005-1.fc21.src.rpm
Description: 
MooX::HandlesVia is an extension of Moo's 'handles' attribute
functionality. It provides a means of proxying functionality from an
external class to the given atttribute. This is most commonly used as a way
to emulate 'Native Trait' behavior that has become commonplace in Moose
code, for which there was no Moo alternative.

Fedora Account System Username: corsepiu

Comment 1 Petr Pisar 2014-03-25 09:09:41 UTC

URL and Source0 are usable. Ok.
Source archive is original (SHA-256: a69de0c201269a4ad2bfce51b200a91bf7f20c0a1d392f130185c8ae533f9360). Ok.
Summary verified from README. Ok.

TODO: I would speel `NativeTrait-like' as `Native-trait-like' or `NativeTraits-like' because `NativeTrait' is neither word nor Moose term (MouseX::NativeTraits).

Description verified from README. Ok.
License verified from README, LICENSE and various ./lib files. Ok.
No XS code, noarch BuildArch is Ok.

TODO: You can replace `PERL_INSTALL_ROOT' with `DESTDIR' argument in the %install section.
TODO: Package either README or README.mkdn. They are redundant.
TODO: Do not package dist.ini. There is nothing spectacular for the end users besides the git URL.

TODO: Constrain `BuildRequire: perl(ExtUtils::MakeMaker)' with `>= 6.30' (Makefile.PL:7).

namespace::autoclean is optional for tests (t/unimport/namespace_autoclean.t:7). Ok.
namespace::clean is optional for tests (t/unimport/namespace_clean.t:7). Ok.

TODO: You should build-require both `perl(namespace::clean)' and `perl(namespace::autoclean)', or none for consistency.

TODO: Build-require `perl(strict)' (Makefile.PL:2).
TODO: Build-require `perl(warnings)' (Makefile.PL:3).

FIX: (Build-)require Data::Perl::* modules instead of Data::Perl itself (Data::Perl::Role::Collection::Array, Data::Perl::Role::Collection::Hash, Data::Perl::Role::String, Data::Perl::Role::Number, Data::Perl::Role::Bool). These are loaded via Moo's `with' keyword.

FIX: Do not Build-require `perl(Role::Tiny)'. It's not used anywhere. It's loaded via Role::Tiny::With.

Data::Dumper is not used (t/from-moose/trait_hash.t:257). Ok.

FIX: Build-require `perl(Moo::Role)' (t/handlesvia_in_role.t:7).

TODO: Build-require `perl(overload)' (t/from-moose/trait_array.t:98).

All tests pass. Ok.

$ rpmlint  perl-MooX-HandlesVia.spec ../SRPMS/perl-MooX-HandlesVia-0.001005-1.fc21.src.rpm ../RPMS/noarch/perl-MooX-HandlesVia-0.001005-1.fc21.noarch.rpm 
perl-MooX-HandlesVia.src: W: spelling-error %description -l en_US proxying -> propping
perl-MooX-HandlesVia.src: W: spelling-error %description -l en_US atttribute -> attribute, attributive, attribution
perl-MooX-HandlesVia.noarch: W: spelling-error %description -l en_US proxying -> propping
perl-MooX-HandlesVia.noarch: W: spelling-error %description -l en_US atttribute -> attribute, attributive, attribution
2 packages and 1 specfiles checked; 0 errors, 4 warnings.
FIX: Correct `atttribute' spelling.

$ rpm -q -lv -p ../RPMS/noarch/perl-MooX-HandlesVia-0.001005-1.fc21.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/doc/perl-MooX-HandlesVia
-rw-r--r--    1 root    root                      700 Dec 20 18:24 /usr/share/doc/perl-MooX-HandlesVia/Changes
-rw-r--r--    1 root    root                    18410 Dec 20 18:24 /usr/share/doc/perl-MooX-HandlesVia/LICENSE
-rw-r--r--    1 root    root                     1430 Dec 20 18:24 /usr/share/doc/perl-MooX-HandlesVia/META.json
-rw-r--r--    1 root    root                     3231 Dec 20 18:24 /usr/share/doc/perl-MooX-HandlesVia/README
-rw-r--r--    1 root    root                     3485 Dec 20 18:24 /usr/share/doc/perl-MooX-HandlesVia/README.mkdn
-rw-r--r--    1 root    root                      242 Dec 20 18:24 /usr/share/doc/perl-MooX-HandlesVia/TODO
-rw-r--r--    1 root    root                      907 Dec 20 18:24 /usr/share/doc/perl-MooX-HandlesVia/dist.ini
-rw-r--r--    1 root    root                     2217 Mar 25 09:33 /usr/share/man/man3/Data::Perl::Bool::MooseLike.3pm.gz
-rw-r--r--    1 root    root                     2453 Mar 25 09:33 /usr/share/man/man3/Data::Perl::Collection::Array::MooseLike.3pm.gz
-rw-r--r--    1 root    root                     2593 Mar 25 09:33 /usr/share/man/man3/Data::Perl::Collection::Hash::MooseLike.3pm.gz
-rw-r--r--    1 root    root                     2219 Mar 25 09:33 /usr/share/man/man3/Data::Perl::Number::MooseLike.3pm.gz
-rw-r--r--    1 root    root                     2600 Mar 25 09:33 /usr/share/man/man3/Data::Perl::String::MooseLike.3pm.gz
-rw-r--r--    1 root    root                     3213 Mar 25 09:33 /usr/share/man/man3/MooX::HandlesVia.3pm.gz
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/Data
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/Data/Perl
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/Data/Perl/Bool
-rw-r--r--    1 root    root                     1308 Dec 20 18:24 /usr/share/perl5/vendor_perl/Data/Perl/Bool/MooseLike.pm
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/Data/Perl/Collection
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/Data/Perl/Collection/Array
-rw-r--r--    1 root    root                     1996 Dec 20 18:24 /usr/share/perl5/vendor_perl/Data/Perl/Collection/Array/MooseLike.pm
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/Data/Perl/Collection/Hash
-rw-r--r--    1 root    root                     2444 Dec 20 18:24 /usr/share/perl5/vendor_perl/Data/Perl/Collection/Hash/MooseLike.pm
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/Data/Perl/Number
-rw-r--r--    1 root    root                     1324 Dec 20 18:24 /usr/share/perl5/vendor_perl/Data/Perl/Number/MooseLike.pm
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/Data/Perl/String
-rw-r--r--    1 root    root                     2229 Dec 20 18:24 /usr/share/perl5/vendor_perl/Data/Perl/String/MooseLike.pm
drwxr-xr-x    2 root    root                        0 Mar 25 09:33 /usr/share/perl5/vendor_perl/MooX
-rw-r--r--    1 root    root                     5509 Dec 20 18:24 /usr/share/perl5/vendor_perl/MooX/HandlesVia.pm
File layout and permissions are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-MooX-HandlesVia-0.001005-1.fc21.noarch.rpm | sort | uniq -c
      1 perl(Class::Method::Modifiers)
      1 perl(Data::Perl) >= 0.002006
      1 perl(:MODULE_COMPAT_5.18.2)
      1 perl(Module::Runtime)
      1 perl(Moo) >= 1.003000
      1 perl(Role::Tiny::With)
      1 perl(strictures) >= 1
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
FIX: Require Data::Perl::* modules instead of Data::Perl itself (Data::Perl::Role::Collection::Array, Data::Perl::Role::Collection::Hash, Data::Perl::Role::String, Data::Perl::Role::Number, Data::Perl::Role::Bool).
The Data::Perl is not used by this RPM package directly. It's used by those modules indirectly. Those modules are loaded by Moo's keyword `with' which is not yet recognized by RPM dependency generator.

$ rpm -q --provides -p ../RPMS/noarch/perl-MooX-HandlesVia-0.001005-1.fc21.noarch.rpm | sort | uniq -c
      1 perl(Data::Perl::Bool::MooseLike) = 0.001005
      1 perl(Data::Perl::Collection::Array::MooseLike) = 0.001005
      1 perl(Data::Perl::Collection::Hash::MooseLike) = 0.001005
      1 perl(Data::Perl::Number::MooseLike) = 0.001005
      1 perl(Data::Perl::String::MooseLike) = 0.001005
      1 perl(MooX::HandlesVia) = 0.001005
      1 perl-MooX-HandlesVia = 0.001005-1.fc21
Binary provides are Ok.

$ resolvedeps f21-build ../RPMS/noarch/perl-MooX-HandlesVia-0.001005-1.fc21.noarch.rpm 
Binary dependencies resolvable. Ok.

Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=6671102). Ok.

Otherwise the package is in line with Fedora and Perl packaging guidelines.

Please correct all `FIX' items, consider fixing `TODO' items, and provide new spec file.
Resolution: Package NOT approved.

Comment 2 Ralf Corsepius 2014-03-25 17:17:10 UTC

(In reply to Petr Pisar from comment #1)

> TODO: I would speel `NativeTrait-like' as `Native-trait-like' or
> `NativeTraits-like' because `NativeTrait' is neither word nor Moose term
> (MouseX::NativeTraits).
Not changed: This is upstream's description, they are using inside of the package and in CPAN
 
> TODO: You can replace `PERL_INSTALL_ROOT' with `DESTDIR' argument in the
> %install section.
I kept PERL_INSTALL_ROOT.

> TODO: Package either README or README.mkdn. They are redundant.
> TODO: Do not package dist.ini. There is nothing spectacular for the end
> users besides the git URL.
Done. This was an oversight on my part.

> TODO: Constrain `BuildRequire: perl(ExtUtils::MakeMaker)' with `>= 6.30'
> (Makefile.PL:7).
Done.

> TODO: You should build-require both `perl(namespace::clean)' and
> `perl(namespace::autoclean)', or none for consistency.
Done. BR'ing both, now.

> TODO: Build-require `perl(strict)' (Makefile.PL:2).
> TODO: Build-require `perl(warnings)' (Makefile.PL:3).
Done.
 
> FIX: (Build-)require Data::Perl::* modules instead of Data::Perl itself
> (Data::Perl::Role::Collection::Array, Data::Perl::Role::Collection::Hash,
> Data::Perl::Role::String, Data::Perl::Role::Number, Data::Perl::Role::Bool).
> These are loaded via Moo's `with' keyword.
Done under protest. All this does it to bloat the dependency tree with redundant deps at no technical benefits.

> FIX: Do not Build-require `perl(Role::Tiny)'. It's not used anywhere. It's
> loaded via Role::Tiny::With.
Done.

> FIX: Build-require `perl(Moo::Role)' (t/handlesvia_in_role.t:7).
Done.

> TODO: Build-require `perl(overload)' (t/from-moose/trait_array.t:98).
Done.


Updated package:
Spec URL: http://corsepiu.fedorapeople.org/packages/perl-MooX-HandlesVia.spec
SRPM URL: http://corsepiu.fedorapeople.org/packages/perl-MooX-HandlesVia-0.001005-2.fc21.src.rpm

Comment 3 Petr Pisar 2014-03-26 14:17:09 UTC

Spec file changes:

diff -u perl-MooX-HandlesVia.spec{.old,}
--- perl-MooX-HandlesVia.spec.old       2014-03-22 05:51:56.000000000 +0100
+++ perl-MooX-HandlesVia.spec   2014-03-25 18:07:47.000000000 +0100
@@ -1,6 +1,6 @@
 Name:           perl-MooX-HandlesVia
 Version:        0.001005
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        NativeTrait-like behavior for Moo
 License:        GPL+ or Artistic
 Group:          Development/Libraries
@@ -9,17 +9,30 @@
 BuildArch:      noarch
 BuildRequires:  perl(Class::Method::Modifiers)
 BuildRequires:  perl(Data::Perl) >= 0.002006
-BuildRequires:  perl(ExtUtils::MakeMaker)
+BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.30
 BuildRequires:  perl(Module::Runtime)
 BuildRequires:  perl(Moo) >= 1.003000
+BuildRequires:  perl(Moo::Role)
 BuildRequires:  perl(MooX::Types::MooseLike::Base) >= 0.23
-BuildRequires:  perl(Role::Tiny)
 BuildRequires:  perl(Role::Tiny::With)
 BuildRequires:  perl(Test::Exception)
 BuildRequires:  perl(Test::Fatal)
 BuildRequires:  perl(Test::More)
 BuildRequires:  perl(namespace::autoclean)
+BuildRequires:  perl(namespace::clean)
+BuildRequires:  perl(overload)
+BuildRequires:  perl(strict)
 BuildRequires:  perl(strictures) >= 1
+BuildRequires:  perl(warnings)
+
+# Redundant to BR: perl(Data::Perl)
+BuildRequires:  perl(Data::Perl::Role::Bool)
+BuildRequires:  perl(Data::Perl::Role::Code)
+BuildRequires:  perl(Data::Perl::Role::Collection::Array)
+BuildRequires:  perl(Data::Perl::Role::Collection::Hash)
+BuildRequires:  perl(Data::Perl::Role::Counter)
+BuildRequires:  perl(Data::Perl::Role::Number)
+BuildRequires:  perl(Data::Perl::Role::String)

 Requires:       perl(Data::Perl) >= 0.002006
 Requires:       perl(Moo) >= 1.003000
@@ -50,10 +63,14 @@
 make test

 %files
-%doc Changes dist.ini LICENSE META.json README README.mkdn TODO
+%doc Changes LICENSE README TODO
 %{perl_vendorlib}/*
 %{_mandir}/man3/*

 %changelog
+* Tue Mar 25 2014 Ralf Corsépius <corsepiu> 0.001005-2
+- Reflect review.
+- Do not package dist.ini, README.mkdn, META.json.
+
 * Fri Mar 21 2014 Ralf Corsépius <corsepiu> 0.001005-1
 - Initial Fedora package.


> TODO: I would speel `NativeTrait-like' as `Native-trait-like' or
> `NativeTraits-like' because `NativeTrait' is neither word nor Moose term (MouseX::NativeTraits).
Not addressed. Ok.

> TODO: You can replace `PERL_INSTALL_ROOT' with `DESTDIR' argument in the
> %install section.
Not addressed. Ok.

> TODO: Package either README or README.mkdn. They are redundant.
> TODO: Do not package dist.ini. There is nothing spectacular for the end users
> besides the git URL.
-%doc Changes dist.ini LICENSE META.json README README.mkdn TODO
+%doc Changes LICENSE README TODO
Ok.

> TODO: Constrain `BuildRequire: perl(ExtUtils::MakeMaker)' with `>= 6.30'
> (Makefile.PL:7).
-BuildRequires:  perl(ExtUtils::MakeMaker)
+BuildRequires:  perl(ExtUtils::MakeMaker) >= 6.30
Ok.

> TODO: You should build-require both `perl(namespace::clean)' and
> `perl(namespace::autoclean)', or none for consistency.
+BuildRequires:  perl(namespace::clean)
Ok.

>TODO: Build-require `perl(strict)' (Makefile.PL:2).
+BuildRequires:  perl(strict)
Ok.

> TODO: Build-require `perl(warnings)' (Makefile.PL:3).
+BuildRequires:  perl(warnings)
Ok.

> FIX: (Build-)require Data::Perl::* modules instead of Data::Perl itself
> (Data::Perl::Role::Collection::Array, Data::Perl::Role::Collection::Hash,
> Data::Perl::Role::String, Data::Perl::Role::Number, Data::Perl::Role::Bool).
> These are loaded via Moo's `with' keyword.
+BuildRequires:  perl(Data::Perl::Role::Bool)
+BuildRequires:  perl(Data::Perl::Role::Code)
+BuildRequires:  perl(Data::Perl::Role::Collection::Array)
+BuildRequires:  perl(Data::Perl::Role::Collection::Hash)
+BuildRequires:  perl(Data::Perl::Role::Counter)
+BuildRequires:  perl(Data::Perl::Role::Number)
+BuildRequires:  perl(Data::Perl::Role::String)
TODO: Do not build-require perl(Data::Perl::Role::Code) and perl(Data::Perl::Role::Counter). These are not used.
TODO: Constrain these dependencies with `>= 0.002006' as was used at perl(Data::Perl).
TODO: Do not build-require `perl(Data::Perl). It's not used.

> FIX: Do not Build-require `perl(Role::Tiny)'. It's not used anywhere. It's loaded
> via Role::Tiny::With.
-BuildRequires:  perl(Role::Tiny)
Ok.

> FIX: Build-require `perl(Moo::Role)' (t/handlesvia_in_role.t:7).
+BuildRequires:  perl(Moo::Role)
Ok.

> TODO: Build-require `perl(overload)' (t/from-moose/trait_array.t:98).
+BuildRequires:  perl(overload)
Ok.

> FIX: Correct `atttribute' spelling.
$ rpmlint perl-MooX-HandlesVia.spec ../SRPMS/perl-MooX-HandlesVia-0.001005-2.fc21.src.rpm ../RPMS/noarch/perl-MooX-HandlesVia-0.001005-2.fc21.noarch.rpm 
perl-MooX-HandlesVia.src: W: spelling-error %description -l en_US proxying -> propping
perl-MooX-HandlesVia.src: W: spelling-error %description -l en_US atttribute -> attribute, attributive, attribution
perl-MooX-HandlesVia.noarch: W: spelling-error %description -l en_US proxying -> propping
perl-MooX-HandlesVia.noarch: W: spelling-error %description -l en_US atttribute -> attribute, attributive, attribution
2 packages and 1 specfiles checked; 0 errors, 4 warnings.
Not address.
TODO: Please replace `atttribute' with `attribute' in the description.

> FIX: Require Data::Perl::* modules instead of Data::Perl itself
> (Data::Perl::Role::Collection::Array, Data::Perl::Role::Collection::Hash,
> Data::Perl::Role::String, Data::Perl::Role::Number, Data::Perl::Role::Bool).
> The Data::Perl is not used by this RPM package directly. It's used by those
> modules indirectly. Those modules are loaded by Moo's keyword `with' which is not
> yet recognized by RPM dependency generator.
$ rpm -q --requires -p ../RPMS/noarch/perl-MooX-HandlesVia-0.001005-2.fc21.noarch.rpm | sort | uniq -c
      1 perl(Class::Method::Modifiers)
      1 perl(Data::Perl) >= 0.002006
      1 perl(:MODULE_COMPAT_5.18.2)
      1 perl(Module::Runtime)
      1 perl(Moo) >= 1.003000
      1 perl(Role::Tiny::With)
      1 perl(strictures) >= 1
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
TODO: Please mirror the Data::Perl::* build-requires into requires. This is how Perl modules dependencies have to be specified in the RPM.

Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=6675583). Ok.

Please consider fixing the `TODO' items before building this package.
Resolution: Package APPROVED.

Comment 4 Ralf Corsepius 2014-03-28 17:59:07 UTC

New Package SCM Request
=======================
Package Name: perl-MooX-HandlesVia
Short Description: NativeTrait-like behavior for Moo
Owners: corsepiu
Branches: f19 f20
InitialCC: perl-sig

Comment 5 Gwyn Ciesla 2014-03-28 18:31:29 UTC

Git done (by process-git-requests).

Comment 6 Ralf Corsepius 2014-03-29 03:53:49 UTC

Package is in rawhide, now.

f20 waits is waiting for perl-Data-Perl to be pushed.

f19 waits for perl-Data-Perl and updated versions of perl-Role-Tiny and perl-Moo.

Comment 7 Fedora Update System 2014-04-18 16:57:48 UTC

perl-MooX-HandlesVia-0.001005-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/perl-MooX-HandlesVia-0.001005-2.fc20

Comment 8 Fedora Update System 2014-04-28 04:27:00 UTC

perl-MooX-HandlesVia-0.001005-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/perl-MooX-HandlesVia-0.001005-2.fc19

Comment 9 Fedora Update System 2014-04-28 04:51:28 UTC

perl-MooX-HandlesVia-0.001005-2.fc20 has been pushed to the Fedora 20 stable repository.

Comment 10 Fedora Update System 2014-05-06 21:33:47 UTC

perl-MooX-HandlesVia-0.001005-2.fc19 has been pushed to the Fedora 19 stable repository.