Bug 483286 - Review Request: perl-Data-Report - A flexible plugin-driven reporting framework
Review Request: perl-Data-Report - A flexible plugin-driven reporting framework
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-30 12:40 EST by Johan Vromans
Modified: 2010-02-04 15:44 EST (History)
7 users (show)

See Also:
Fixed In Version: 0.10-4.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-05-15 19:26:16 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Johan Vromans 2009-01-30 12:40:07 EST
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-1.fc11.src.rpm
Description:
Data::Report is a framework for report generation.

You define the columns, add the data row by row, and get reports in
text, HTML, CSV and so on. Textual ornaments like extra empty lines,
dashed lines, and cell lines can be added in a way similar to HTML
style sheets.

Last koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1094325

This is one of my first packages, so I'm looking for a sponsor.
The spec and srpm are rpmlint free of warnings and errors.
Comment 1 Parag AN(पराग) 2009-01-31 01:29:10 EST
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.
Comment 2 Ralf Corsepius 2009-01-31 01:51:38 EST
I am inclined to think, we first need to have Text::CSV in Fedora, before this package can be added.
Comment 3 Johan Vromans 2009-01-31 05:37:33 EST
@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.
Comment 4 Ralf Corsepius 2009-01-31 06:43:22 EST
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
Comment 5 Johan Vromans 2009-01-31 08:04:39 EST
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.
Comment 6 Hans de Goede 2009-04-03 07:06:34 EDT
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.
Comment 7 Johan Vromans 2009-04-13 10:08:45 EDT
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.
Comment 8 Hans de Goede 2009-04-16 05:33:57 EDT
Looks good, approved! Once eekboek also is in good shape I'll sponsor you.
Comment 9 Johan Vromans 2009-04-22 07:47:43 EDT
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:
Comment 10 Kevin Fenzi 2009-04-23 12:25:21 EDT
cvs done.
Comment 11 Fedora Update System 2009-04-27 15:11:53 EDT
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
Comment 12 Fedora Update System 2009-04-27 15:11:59 EDT
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
Comment 13 Fedora Update System 2009-04-27 21:22:41 EDT
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
Comment 14 Fedora Update System 2009-05-09 00:04:17 EDT
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
Comment 15 Fedora Update System 2009-05-15 19:26:11 EDT
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.
Comment 16 Fedora Update System 2009-05-15 19:26:53 EDT
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.
Comment 17 Michal Ingeli 2009-12-28 07:31:56 EST
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.
Comment 18 Johan Vromans 2009-12-28 13:37:38 EST
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).
Comment 19 Kevin Fenzi 2009-12-28 22:02:03 EST
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?
Comment 20 Michal Ingeli 2009-12-29 03:11:44 EST
(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.
Comment 21 Johan Vromans 2009-12-29 04:04:24 EST
That's fine with me. It might be a good idea to have a separate maintainer for the EL branches.
Comment 22 Kevin Fenzi 2010-01-02 15:22:15 EST
cvs done.
Comment 23 Fedora Update System 2010-01-19 05:53:26 EST
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
Comment 24 Fedora Update System 2010-02-04 15:43:52 EST
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.

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