Bug 579682 - Review Request: perl-version - Perl extension for Version Objects
Summary: Review Request: perl-version - Perl extension for Version Objects
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Iain Arnell
QA Contact: Fedora Extras Quality Assurance
Depends On:
Blocks: 569361 perl-duallifed
TreeView+ depends on / blocked
Reported: 2010-04-06 09:10 UTC by Marcela Mašláňová
Modified: 2010-05-11 06:58 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2010-05-11 06:58:17 UTC
iarnell: fedora-review+
kevin: fedora-cvs+

Attachments (Terms of Use)

Description Marcela Mašláňová 2010-04-06 09:10:42 UTC
Spec URL: http://mmaslano.fedorapeople.org/review/perl-version.spec
SRPM URL: http://mmaslano.fedorapeople.org/review/perl-version-0.80-1.fc13.src.rpm
Version objects were added to Perl in 5.10. This module implements version 
objects for older version of Perl and provides the version object API for  
all versions of Perl. All previous releases before 0.74 are deprecated and  
should not be used due to incompatible API changes. Version 0.77 introduces 
the new 'parse' and 'declare' methods to standardize usage. You are 
strongly urged to set 0.77 as a minimum in your code, e.g.

Comment 1 Marcela Mašláňová 2010-04-06 09:12:12 UTC
Previous review ticket #177038 is quite old. We need to reopen cvs for this module after the module will be reviewed and also unblock this package from collections.

Comment 2 Iain Arnell 2010-05-02 07:02:02 UTC
+ source files match upstream.  
    0add2a0132d582ad81d62b4e38fd3f92  version-0.80.tar.gz

+ package meets naming and versioning guidelines.
+ specfile is properly named, is cleanly written and uses macros consistently.
+ summary is OK.
+ description is OK.
+ dist tag is present.
+ build root is OK.
+ license field matches the actual license.
    GPL+ or Artistic

+ license is open source-compatible.
+ license text not included upstream.
- latest version is being packaged.
    version-0.82 is available

- BuildRequires are proper.
    missing perl(Module::Build)

+ compiler flags are appropriate.
+ %clean is present.
+ package builds in mock (after adding missing BR)

+ package installs properly.
+ rpmlint has no complaints:
    perl-version.src: I: checking
    perl-version.src: I: checking-url http://search.cpan.org/dist/version/ (timeout 10 seconds)
    perl-version.src: I: checking-url http://www.cpan.org/authors/id/J/JP/JPEACOCK/version-0.80.tar.gz (timeout 10 seconds)
    perl-version.x86_64: I: checking
    perl-version.x86_64: I: checking-url http://search.cpan.org/dist/version/ (timeout 10 seconds)
    perl-version-debuginfo.x86_64: I: checking
    perl-version-debuginfo.x86_64: I: checking-url http://search.cpan.org/dist/version/ (timeout 10 seconds)
    3 packages and 0 specfiles checked; 0 errors, 0 warnings.

+ final provides and requires are sane:
    perl(version) = 0.80
    perl(version::vxs) = 0.80
    perl-version = 0.80-1.fc14
    perl-version(x86-64) = 0.80-1.fc14
    perl >= 0:5.005_03
    perl >= 0:5.005_04
    rpmlib(CompressedFileNames) <= 3.0.4-1
    rpmlib(FileDigests) <= 4.6.0-1
    rpmlib(PayloadFilesHavePrefix) <= 4.0-1
    rpmlib(VersionedDependencies) <= 3.0.3-1
    rpmlib(PayloadIsXz) <= 5.2-1

+ %check is present and all tests pass.
    ./Build test
    t/01base.t ..... ok
    t/02derived.t .. ok
    t/03require.t .. ok
    All tests successful.
    Files=3, Tests=1382,  0 wallclock secs ( 0.15 usr  0.03 sys +  0.51 cusr  0.05 csys =  0.74 CPU)
    Result: PASS

+ no shared libraries are added to the regular linker search paths.
+ owns the directories it creates.
+ doesn't own any directories it shouldn't.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no generically named files
+ code, not content.
+ documentation is small, so no -doc subpackage is necessary.
+ %docs are not necessary for the proper functioning of the package.

Looks good - just a couple of little points.

You've commented out the BuildRequires: perl(Module::Build), but it's really necessary.

There's a newer version-0.82 available now.

And you should probably use %{?perl_default_filter} to avoid providing vxs.so.

With those tweaks, APPROVED.

Comment 3 Iain Arnell 2010-05-02 07:47:19 UTC
Oops. Missed one. You also need Epoch: 3 to match that of perl-version from core.

Comment 4 Marcela Mašláňová 2010-05-03 06:23:00 UTC
Thanks for review, everything fixed:

New Package CVS Request
Package Name: perl-version
Short Description: Perl extension for Version Objects
Owners: mmaslano
Branches: F-12 F-13
InitialCC: perl-sig

Comment 5 Kevin Fenzi 2010-05-04 03:03:54 UTC
Spot owns this package in other existing branches. Adding him here to CC. 

Is it ok to branch this for F-12/F-13 with Marcela as owner?

Comment 6 Marcela Mašláňová 2010-05-04 07:03:45 UTC
(In reply to comment #5)
> Spot owns this package in other existing branches. Adding him here to CC. 
> Is it ok to branch this for F-12/F-13 with Marcela as owner?    

I don't mind if spot takes also the new branches ;-)

Comment 7 Tom "spot" Callaway 2010-05-04 13:45:07 UTC
Go ahead and branch it, I'm happy to share it with Marcela.

Comment 8 Kevin Fenzi 2010-05-06 15:50:00 UTC
cvs done.

Comment 9 Marcela Mašláňová 2010-05-07 06:14:31 UTC
Please unblock this package in cvs.

Comment 10 Kevin Fenzi 2010-05-07 16:27:07 UTC
I can't do that, I'm not in rel-eng. ;) 

Please file a request in the rel-eng track asking for it to be unblocked?

Comment 11 Marcela Mašláňová 2010-05-10 09:02:33 UTC

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