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 233585 - Review Request: perl-XML-Writer - A simple Perl module for writing XML documents
Summary: Review Request: perl-XML-Writer - A simple Perl module for writing XML documents
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-23 11:07 UTC by Alex Lancaster
Modified: 2010-09-05 17:32 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-03-27 01:56:57 UTC
panemade: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Alex Lancaster 2007-03-23 11:07:43 UTC
Spec URL: http://allele5.biol.berkeley.edu/~alex/fedora/perl-XML-Writer.spec
SRPM URL: http://allele5.biol.berkeley.edu/~alex/fedora/perl-XML-Writer-0.602-1.src.rpm here>
Description: 
XML::Writer is a simple Perl module for writing XML documents: it
takes care of constructing markup and escaping data correctly, and by
default, it also performs a significant amount of well-formedness
checking on the output, to make certain (for example) that start and
end tags match, that there is exactly one document element, and that
there are not duplicate attribute names.

Comment 1 Parag AN(पराग) 2007-03-23 11:19:40 UTC
Kindly avoid hardcoding version number in Source URL tarball name. Instead use
%{version}
Anyway you can do that at time of importing package in CVS.

Comment 2 Parag AN(पराग) 2007-03-23 11:20:25 UTC
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream url
c715d6fd90ac775316cc313815ba3b77  XML-Writer-0.602.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no headers or static libraries.
+ no .pc file present.
+ no -devel subpackage
+ no .la files.
+ no translations are available
+ Does owns the directories it creates.
+ no scriptlets present.
+ no duplicates in %files.
+ file permissions are appropriate.
+ make test
PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0,
'blib/lib', 'blib/arch')" t/*.t
t/01_main.........ok
t/pod-coverage....skipped
        all skipped: Test::Pod::Coverage required for testing pod coverage
t/pod.............skipped
        all skipped: Test::Pod 1.00 required for testing POD
All tests successful, 2 tests skipped.
Files=3, Tests=213,  1 wallclock secs ( 0.30 cusr +  0.03 csys =  0.33 CPU)

+ Provides: perl(XML::Writer) = 0.602 perl(XML::Writer::Namespaces)
perl(XML::Writer::_String)
APPROVED.


Comment 3 Ralf Corsepius 2007-03-23 12:11:08 UTC
Pushing back to review, because of this:

1.)

t/pod-coverage....skipped
        all skipped: Test::Pod::Coverage required for testing pod coverage
t/pod.............skipped
        all skipped: Test::Pod 1.00 required for testing POD

=>

BR: perl(Test::Pod::Coverage)
BR: perl(Test::Pod)


2.
BuildRequires:  perl >= 1:5.6.1
=> Superfluous

3. Missing
BuildRequires: perl(ExtUtils::MakeMaker)

Parag, Alex, these remarks also seem to apply to most other perl-packages Parag
"nodded off" today.

Comment 4 Ville Skyttä 2007-03-23 17:52:55 UTC
On a related note, I don't think testing for coverage of POD documentation is
really that interesting at all from packaging point of view.  If the
documentation coverage test fails, would you block a package from inclusion or
being updated?

So opting to explicity disable the POD coverage tests would IMO be actually even
better than making sure that the associated build dependencies are around and
that the tests are always run (which is acceptable too; the current non-explicit
state of this package is not).  Note: I'm talking about POD coverage tests only
here, not about any other tests.

Comment 6 Parag AN(पराग) 2007-03-26 05:49:54 UTC
(In reply to comment #3)
> Pushing back to review, because of this:
> 
> 1.)
> 
> t/pod-coverage....skipped
>         all skipped: Test::Pod::Coverage required for testing pod coverage
> t/pod.............skipped
>         all skipped: Test::Pod 1.00 required for testing POD
> 
> =>
> 
> BR: perl(Test::Pod::Coverage)
> BR: perl(Test::Pod)
> 
   I thought its not much important as its only check section that said about
skipping testing.
> 
> 2.
> BuildRequires:  perl >= 1:5.6.1
> => Superfluous
> 
  Oops its not needed. Missed one ;)

> 3. Missing
> BuildRequires: perl(ExtUtils::MakeMaker)
    yes
> 
> Parag, Alex, these remarks also seem to apply to most other perl-packages Parag
> "nodded off" today.

Got some spare time to review packages :)

Comment 7 Parag AN(पराग) 2007-03-26 06:20:59 UTC
(In reply to comment #3)
> Pushing back to review, because of this:
> 
> 1.)
> 
> t/pod-coverage....skipped
>         all skipped: Test::Pod::Coverage required for testing pod coverage
> t/pod.............skipped
>         all skipped: Test::Pod 1.00 required for testing POD
> 
> =>
> 
> BR: perl(Test::Pod::Coverage)
> BR: perl(Test::Pod)
> 
> 
> 2.
> BuildRequires:  perl >= 1:5.6.1
> => Superfluous
> 
> 3. Missing
> BuildRequires: perl(ExtUtils::MakeMaker)
> 
Do we have any wiki page that gives above information for perl package
reviewers/packagers?

> Parag, Alex, these remarks also seem to apply to most other perl-packages Parag
> "nodded off" today.



Comment 8 Parag AN(पराग) 2007-03-26 06:26:28 UTC
With new package SRPM
+make test
PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0,
'blib/lib', 'blib/arch')" t/*.t
t/01_main.........ok
t/pod-coverage....ok
t/pod.............ok
All tests successful.
Files=3, Tests=215,  1 wallclock secs ( 0.46 cusr +  0.04 csys =  0.50 CPU)
+ exit 0

All other looks OK to me.

Comment 9 Ralf Corsepius 2007-03-26 06:28:25 UTC
(In reply to comment #4)
> On a related note, I don't think testing for coverage of POD documentation is
> really that interesting at all from packaging point of view. 
Right, but it has always been our policy to enable them, when a perl-dist's test
suite uses them (More precisely: It has been policy to check everything a
test-suite can test for) 

I don't see any reason to make any exceptions.

(In reply to comment #6)
> (In reply to comment #3)
>    I thought its not much important as its only check section that said about
> skipping testing.
Checks aren't "only". We should "test to the max", if possible.

However you are right, Test::Pods aren't very important, but ... c.f. above.

Finally, another remark:
%check ||:

Remove the ||: - This is an anachronism to please "really ancient rpms" and has
been frowned upon for years. Use %check.


Comment 10 Parag AN(पराग) 2007-03-26 06:44:25 UTC
update SPEC and resubmit package removing ||:


Comment 12 Parag AN(पराग) 2007-03-26 08:50:03 UTC
Thanks.
APPROVED Again.

Comment 13 Alex Lancaster 2007-03-26 08:56:22 UTC
New Package CVS Request
=======================
Package Name: perl-XML-Writer
Short Description: A simple Perl module for writing XML documents
Owners: alexl@users.sourceforge.net
Branches: FC-5 FC-6
InitialCC: 

Another orphaned package being resurrected.

Comment 14 Alex Lancaster 2007-03-27 01:56:57 UTC
Builds fine in devel, FC-6, FC-5.  Closing bug.

Comment 15 Xavier Bachelot 2008-04-22 08:16:12 UTC
Hi Alex,

As discussed by mail, I'll take EPEL branches ownership.

Regards,
Xavier

Package Change Request
======================
Package Name: perl-XML-Writer
New Branches: EL-4 EL-5

Updated EPEL Owners: xavierb
Updated EPEL CC: xavierb alexlan

Comment 16 Xavier Bachelot 2008-04-22 08:28:53 UTC
Doh, I didn't add Alex to the owners...
Corrected CVS request below :

Package Change Request
======================
Package Name: perl-XML-Writer
New Branches: EL-4 EL-5

Updated EPEL Owners: xavierb alexlan
Updated EPEL CC: xavierb alexlan

Comment 17 Kevin Fenzi 2008-04-22 17:07:23 UTC
cvs done.

Comment 18 Xavier Bachelot 2008-04-22 19:04:17 UTC
built for EL-4 and EL-5.

Comment 19 Mark Chappell 2010-09-03 08:30:59 UTC
Package Change Request
======================
Package Name: perl-XML-Writer
New Branches: EL-6
Owners: xavierb alexlan tremble iarnell 

Yet another some arches only perl package.

08:26 < tremble> schlobinux_ : Ping - Any objections to perl-XML-Writer getting branched for EL-6, it's yet another some arches only package.
08:27 < schlobinux_> tremble: no objection, indeed.

Comment 20 Kevin Fenzi 2010-09-05 17:32:12 UTC
Git done (by process-git-requests).


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