Bug 483286

1)Add following to spec
BuildRequires:  perl(HTML::Entities)
Requires:       perl(HTML::Entities)

2) %doc is empty it should be
%doc Changes README

3) Not needed to use full path to find command.

I am inclined to think, we first need to have Text::CSV in Fedora, before this package can be added.

@Parag: Thanks for your feedback.
1) HTML::Entities is an optional module and not necessary for building and correct functioning of this package. Do you still need it should be added to the requirements?
2) Fixed.
3) Fixed.

@Ralf: I do not understand. The package can work with either Text::CSV or Text::CSV_XS. The current version mentions Text::CSV in its requirements, and I changed that to Text::CSV_XS mainly because Text::CVS_XS is part of Fedora already.

In my understanding, Text::CSV is an abstraction layer, which may use Text::CVS_XS underneath.

That said, your patch to me is a hack to circumvent Text::CSV.

=> The proper way to package would be to first package Text::CSV, and then Data::Report

Thanks for your clarification.
Not so long ago, Text::CSV and Text::CSV_XS were incompatible implementations with differing APIs. That is why many many tools require Text::CSV_XS directly. And I assume that is also the reason why Text::CSV_XS has been part of Fedora for a long time, while Text::CSV still isn't.
Only recently (2007/2008) development of Text::CSV and Text::CSV_XS was coordinated.
So I do not share your opinion on my patch being a hack.
However, I have contacted the Text::CSV maintainer to ask permission for packaging.

Hi Johan,

As discussed by mail already, I'll review your current 4 package submissions, and
once they are all approved, I'll sponsor you.

I've done a full review of this package all in all it looks good, except
for a few small things (see below).

MUST FIX
--------
* Drop the patch and (Build)Requires perl(Text::CSV) instead of
  Text::CSV_XS
  I know you are upstream, but we do not want to deviate from upstream when
  not necessary and since you've now also packaged TEXT::CSV, there is no
  reason for the patch anymore.
* Add Changes and README to %doc

SHOULD FIX
----------
* Add following to spec:
BuildRequires:  perl(HTML::Entities)
Requires:       perl(HTML::Entities)

I know this is optional but in Fedora we always try to enable as much
features in packaes as possible, even if this drags in a few additional
dependencies.

Spec URL: http://www.squirrel.nl/pub/xfer/perl-Data-Report.spec
SRPM URL: http://www.squirrel.nl/pub/xfer/perl-Data-Report-0.10-2.fc10.src.rpm

This version fixes the remarks (MUST FIX and SHOULD FIX) from the reviewers, as summarized in #6.

No koji build, since it now requires perl-Text-CSV which is not yet part of Fedora.

The package and .spec are rpmlint free except for a harmless warning.

Looks good, approved! Once eekboek also is in good shape I'll sponsor you.

New Package CVS Request
=======================
Package Name: perl-Data-Report
Short Description: A flexible plugin-driven reporting framework
Owners: sciurius
Branches: F-10 F-11
InitialCC:

cvs done.

perl-Data-Report-0.10-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/perl-Data-Report-0.10-1.fc10

perl-Data-Report-0.10-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/perl-Data-Report-0.10-1.fc11

perl-Data-Report-0.10-1.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update perl-Data-Report'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-4085

perl-Data-Report-0.10-1.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update perl-Data-Report'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-4421

perl-Data-Report-0.10-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

perl-Data-Report-0.10-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: perl-Data-Report
New Branches: EL-5
Owners: sciurius ksyz

The EL-5 branch is needed for MySQL-zrm in EPEL. There is a report (#550192), that MySQL-zrm wont install on el5 and in fact, it misses dependencies like perl(Data::Report) and perl(Text::CSV) that are not branched and build for epel.

I sent notification and request for EL-5 branch to he package owner few day ago. For owner, feel free to take over this branch.

Package builds in mock without problem.

I do not have any particular interest in EPEL. I don't mind building for EPEL either provided someone can tell me what to do (in trivial steps).

See: https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies

The infrastructure is all the same as the F-* branches... just need to be more conservative pushing updates and new versions. 

Shall I process the request from comment 17?

(In reply to comment #18)
> I do not have any particular interest in EPEL. I don't mind building for EPEL
> either provided someone can tell me what to do (in trivial steps).

Packages are handled more sensitively, tested, trying not to break things and change API and so.

I'm willing to maintain the EPEL branch.

(In reply to comment #19)
> Shall I process the request from comment 11?  

If there is no problem, please proceed.

That's fine with me. It might be a good idea to have a separate maintainer for the EL branches.

cvs done.

perl-Data-Report-0.10-4.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/perl-Data-Report-0.10-4.el5

perl-Data-Report-0.10-4.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.