[Bug 992951] Review Request: perl-MooX-Types-MooseLike - Some Moosish types and a type builder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=992951

Paul Howarth <paul@xxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |paul@xxxxxxxxxxxx
       Docs Contact|                            |paul@xxxxxxxxxxxx
              Flags|                            |fedora-review+



--- Comment #1 from Paul Howarth <paul@xxxxxxxxxxxx> ---
rpmlint output
==============
$ rpmlint ~/perl-MooX-Types-MooseLike-0.25-1.fc20.*
perl-MooX-Types-MooseLike.noarch: W: spelling-error %description -l en_US
parameterizable -> parameter
perl-MooX-Types-MooseLike.src: W: spelling-error %description -l en_US
parameterizable -> parameter
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

Nothing to worry about there.

requires
========
perl(:MODULE_COMPAT_5.18.0)
perl(Carp)
perl(Exporter) >= 5.57
perl(List::Util)
perl(Module::Runtime)
perl(Module::Runtime) >= 0.012
perl(MooX::Types::MooseLike)
perl(Scalar::Util)
perl(strict)
perl(warnings)
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1

provides
========
perl(MooX::Types::MooseLike) = 0.25
perl(MooX::Types::MooseLike::Base) = 0.25
perl-MooX-Types-MooseLike = 0.25-1.fc20

review checklist
================

- rpmlint OK
- package and spec file naming OK
- package meets guidelines
- license is same as perl, OK for Fedora and correctly specified in spec
- upstream did not include a license file to package
- spec file written in English and is legible
- source file matches upstream
- package builds OK in mock for Rawhide x86_64
- buildreqs mostly OK but see below
- no libraries, locale data, devel files etc. to worry about
- package is not intended to be relocatable
- directory ownership OK
- no duplicate files
- permissions are sane
- macro usage is consistent
- code, not content
- no large docs
- docs don't affect runtime
- not a GUI app, no desktop file needed
- all filenames are ASCII
- no scriptlets present or needed
- no subpackages present or needed
- requires/provides OK

Nits:

The Test::More version requirement of this package means that it won't even
build for EPEL-6,
so there's no point in including various bits of boilerplate that are only
needed for EPEL-5
support:

 * BuildRoot specification
 * Emptying the buildroot at the start of %install
 * Having a %clean section
 * The default %defattr(-,root,root,-) isn't even needed for EPEL-5

There's never a need to remove empty directories from the buildroot.

I don't really see the value to end users of including META.json.

The usual convention these days is to use DESTDIR rather than
PERL_INSTALL_ROOT.

If you want to explicitly require Module::Runtime ≥ 0.012, you should probably
filter the automatically-generated dependency on Module::Runtime (any version):
%global __provides_exclude ^perl\\(Module::Runtime\\)$

For completeness, the following are also needed as they are used either by the
module
or its test suite:

BuildRequires: perl(Carp)
BuildRequires: perl(Exporter) >= 5.57
BuildRequires: perl(IO::Handle)
BuildRequires: perl(List::Util)
BuildRequires: perl(Moo::Role)
BuildRequires: perl(overload)
BuildRequires: perl(Role::Tiny)
BuildRequires: perl(Scalar::Util)
BuildRequires: perl(strict)
BuildRequires: perl(warnings)

There's also an argument for having a runtime dependency on
perl(Moose::Util::TypeConstraints), but as that would pull in Moose, there's
also a good argument for not including it. It's probably better for users that
need that functionality to require it themselves.


None of these are blockers.

APPROVED.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=txVaRl8muM&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]