Bug 1074482 - Review Request: perl-Net-Twitter-Lite - Perl interface to the Twitter API
Summary: Review Request: perl-Net-Twitter-Lite - Perl interface to the Twitter API
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Paul Howarth
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-03-10 11:02 UTC by David Dick
Modified: 2014-04-06 18:54 UTC (History)
2 users (show)

Fixed In Version: perl-Net-Twitter-Lite-0.12006-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-03-31 02:09:58 UTC
Type: ---
Embargoed:
paul: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description David Dick 2014-03-10 11:02:33 UTC
Spec URL: http://ddick.fedorapeople.org/packages/perl-Net-Twitter-Lite.spec
SRPM URL: http://ddick.fedorapeople.org/packages/perl-Net-Twitter-Lite-0.12006-1.fc20.src.rpm
Description: Perl interface to the Twitter API
Fedora Account System Username: ddick

Comment 1 David Dick 2014-03-10 11:07:23 UTC
koji build at http://koji.fedoraproject.org/koji/taskinfo?taskID=6616731

Comment 2 Paul Howarth 2014-03-20 13:42:25 UTC
Review checklist
================

- rpmlint clean
- package and spec file naming OK
- package meets guidelines
- license is same as perl, matches upstream, and is OK for Fedora
- upstream's license file is packaged
- spec file written in English and is legible
- source file matches upstream
- package builds OK is mock for EPEL-6 (x86_64) and in koji for F-21 (x86_64)
- build dependencies mostly ok (see below)
- no locale data or shared libraries to concern ourselves with
- no bundled libraries included
- package is not intended to be relocatable
- directory ownership is fine
- no duplicate files
- permissions are fine
- macro usage is consistent
- code, not content
- no large docs
- docs shouldn't affect runtime
- no static libraries, development files or sub-packages to worry about
- not a GUI app, no desktop file needed
- all filenames are ASCII
- no scriptlets
- no pkgconfig file needed

TODO
====

perl(Module::Build) version requirement should be 0.35, as per your patch.

I don't see where perl(base), perl(File::Spec), perl(Scalar::Util),
perl(Storable) and perl(vars) are used, except in example files and/or
documentation, so they're not really needed as buildreqs or (in some cases)
explicit runtime deps.

Similarly, Data::Dumper is used by the test suite and in an example file,
but there's no real runtime need for it, so the explicit require of it is
unnecessary.

Upstream has manually added Test::Fatal as a test dependency but doesn't
actually use it, so it could be removed. They also specified Test::Simple
0.98 but as you've found, older versions work fine. Looking at the tests,
I think Test::More version 0.82 is the actual requirement, for note() in
t/00-compile.t. I'd drop the Test::Simple dependency and add the version
dependency for perl(Test::More).

Nits
====

You have the build requirements for the release tests, but don't run them;
I'd suggest either dropping those buildreqs or running the release tests.

Since you've added an explicit runtime dependency on perl(Net::Netrc), you
should probably add a BuildRequires for it too. Unless there's a bootstrapping
issue, I think it's good practice to build-require everything that's required
at runtime, which can help you find some dependency issues at build-time that
you wouldn't otherwise discover until the built package is pushed to a repo
and someone tries to install it.

The dependency filtering is usually placed just before %description in the
spec.
https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Location_of_macro_invocation
I'd add a comment that your filtering is to remove under-specified
dependencies, since you're explictly requiring particular versions of
those modules.

I'd put %{perl_vendorlib}/Net/ instead of %{perl_vendorlib}/* in the %files
list; there's really no need for a wildcard there.

Summary
=======

Package has been put together carefully and isn't just the output of cpanspec;
none of the issues above are blockers, so feel free to address them or not as
you see fit.

APPROVED.

Comment 3 David Dick 2014-03-21 06:58:41 UTC
New Package SCM Request
=======================
Package Name: perl-Net-Twitter-Lite
Short Description: Perl interface to the Twitter API
Owners: ddick
Branches: f20 el6 epel7
InitialCC: perl-sig

Comment 4 David Dick 2014-03-21 09:15:40 UTC
Good catch. :)

Comment 5 Gwyn Ciesla 2014-03-21 12:00:01 UTC
Git done (by process-git-requests).

Comment 6 Fedora Update System 2014-03-22 02:32:09 UTC
perl-Net-Twitter-Lite-0.12006-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/perl-Net-Twitter-Lite-0.12006-1.el6

Comment 7 Fedora Update System 2014-03-22 02:32:59 UTC
perl-Net-Twitter-Lite-0.12006-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/perl-Net-Twitter-Lite-0.12006-1.fc20

Comment 8 Fedora Update System 2014-03-22 22:28:26 UTC
perl-Net-Twitter-Lite-0.12006-1.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 9 Fedora Update System 2014-03-31 02:09:58 UTC
perl-Net-Twitter-Lite-0.12006-1.fc20 has been pushed to the Fedora 20 stable repository.

Comment 10 Fedora Update System 2014-04-06 18:54:36 UTC
perl-Net-Twitter-Lite-0.12006-1.el6 has been pushed to the Fedora EPEL 6 stable repository.


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