Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 452523 - Review Request: perl-Event-ExecFlow - High level API for event-based execution flow control
Summary: Review Request: perl-Event-ExecFlow - High level API for event-based executio...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Xavier Bachelot
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 452516
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-06-23 15:00 UTC by Nicolas Chauvet (kwizart)
Modified: 2008-07-24 22:19 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-07-24 22:19:44 UTC
xavier: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Nicolas Chauvet (kwizart) 2008-06-23 15:00:15 UTC
Spec URL:
http://kwizart.fedorapeople.org/SPECS/perl-Event-ExecFlow.spec
SRPM URL:
http://kwizart.fedorapeople.org/SRPMS/perl-Event-ExecFlow-0.63-1.fc8.kwizart.src.rpm
Description: High level API for event-based execution flow control

Comment 1 Xavier Bachelot 2008-07-10 20:25:24 UTC
missing BuildRequires :
Warning: prerequisite Locale::TextDomain 0 not found.
Warning: prerequisite Test::More 0 not found.


Comment 2 Nicolas Chauvet (kwizart) 2008-07-10 20:43:43 UTC
Spec URL:
http://kwizart.fedorapeople.org/SPECS/perl-Event-ExecFlow.spec
SRPM URL:
http://kwizart.fedorapeople.org/SRPMS/perl-Event-ExecFlow-0.63-2.fc8.kwizart.src.rpm
Description: High level API for event-based execution flow control

changelog
- Add BR Test::More and Locale::TextDomain

Comment 3 Xavier Bachelot 2008-07-11 22:50:29 UTC
+ source files match upstream : 79116732b550701a3436a448581e01da
+ package meets naming and versioning guidelines.
+ specfile is properly named, is cleanly written and uses macros consistently.
+ dist tag is present.
+ build root is correct.

- license field matches the actual license :
The README file says same license as perl, thus GPL+ or Artistic, but most files
say LGPLv2+

+ license is open source-compatible. License text not included upstream.
+ latest version is being packaged.
+ BuildRequires are proper.
+ compiler flags are appropriate.
+ %clean is present.
+ package builds in mock ( ).
+ package installs properly
- rpmlint is silent :
perl-Event-ExecFlow.noarch: E: non-standard-executable-perm /usr/bin/execflow 0555
perl-Event-ExecFlow.noarch: W: file-not-utf8
/usr/share/doc/perl-Event-ExecFlow-0.63/README

- final provides and requires are sane:
This provide is dubious :
perl(AnyEvent::Impl::Event::Glib)

+ %check is present and all tests pass
+ owns the directories it creates.

- doesn't own any directories it shouldn't.
The directories below are installed and owned :
/usr/lib/perl5/vendor_perl/5.10.0/i386-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.10.0/i386-linux-thread-multi/auto
/usr/lib/perl5/vendor_perl/5.10.0/i386-linux-thread-multi/auto/Event
/usr/lib/perl5/vendor_perl/5.10.0/i386-linux-thread-multi/auto/Event/ExecFlow

+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets present.
+ code, not content.
+ documentation is small, so no -docs subpackage is necessary.
+ %docs are not necessary for the proper functioning of the package.

Comment 4 Nicolas Chauvet (kwizart) 2008-07-15 22:30:32 UTC
Spec URL:
http://kwizart.fedorapeople.org/SPECS/perl-Event-ExecFlow.spec
SRPM URL:
http://kwizart.fedorapeople.org/SRPMS/perl-Event-ExecFlow-0.63-3.fc8.kwizart.src.rpm
Description: High level API for event-based execution flow control

Changelog
- Fix directory ownership
- Fix execflow perm
- Fix perl Encoding
- Fix License to LGPLv2+

And fix AnyEvent::Impl::Event::Glib wrong provides as it was already provided by
perl-AnyEvent (good catch).



Comment 5 Xavier Bachelot 2008-07-17 12:50:39 UTC
Looks good, the last remaining point is the license. As I previously noted, all
.pm files are LGPLv2+, only lib/Event/ExecFlow.pm is GPL+ or Artistic. The
README also says GPL+ or Artistic. I'm not sure this makes the whole package
LGPLv2+, but my license-fu is weak.

Comment 6 Ralf Corsepius 2008-07-17 15:29:52 UTC
Well, each source file applies a license of it's own, with inlined licenses
overruling detached licenses.

I.e. as all *.pm's carry an explicit license, the global README is mostly void.
=> This package contains both "GPL+ or Artistic" files and "LGPL'ed" files.

Now the big (and controversal) question is: 
a) Are perl-modules linked or 
b) are they simply calling each other?

If a) applies, the only way to ship this package is to relicense it as
"GPL+"-only (LGPL'ed code can be changed to GPL)
If b) applies, this discussion is moot.

I think b) applies, but IANAL.



Comment 7 Nicolas Chauvet (kwizart) 2008-07-20 21:05:57 UTC
So, what to do with this ? I think it would do a 
(GPL+ or Artistic) and LGPLv2+

with a note like:
# This file is GPL+ or Artistic
%{_bindir}/execflow
# Theses files are LGPLv2+
%{perl_vendorlib}/Event/
%{_mandir}/man3/*.3*



Comment 8 Xavier Bachelot 2008-07-23 19:16:32 UTC
(In reply to comment #7)
> So, what to do with this ? I think it would do a 
> (GPL+ or Artistic) and LGPLv2+
> 
> with a note like:
> # This file is GPL+ or Artistic
> %{_bindir}/execflow
> # Theses files are LGPLv2+
> %{perl_vendorlib}/Event/
> %{_mandir}/man3/*.3*
> 
> 
I've checked the licensing guidelines and with this change the package is APPROVED.


Comment 9 Nicolas Chauvet (kwizart) 2008-07-24 07:09:43 UTC
New Package CVS Request
=======================
Package Name: perl-Event-ExecFlow
Short Description: High level API for event-based execution flow control
Owners: kwizart
Branches: F-8 F-9 EL-4 EL-5
InitialCC: perl-sig
Cvsextras Commits: yes

Comment 10 Kevin Fenzi 2008-07-24 18:14:27 UTC
cvs done.


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