Bug 231303 - Review Request: xmlunit - Provides classes to do asserts on xml
Summary: Review Request: xmlunit - Provides classes to do asserts on xml
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matt Wringe
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-07 16:14 UTC by Permaine Cheung
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-07-09 16:03:22 UTC
Type: ---
Embargoed:
mwringe: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Permaine Cheung 2007-03-07 16:14:17 UTC
Spec URL: https://pcheung.108.redhat.com/files/documents/174/269/xmlunit.spec
SRPM URL: http://mirrors.dotsrc.org/jpackage/1.7/generic/free/SRPMS/xmlunit-1.0-4jpp.src.rpm
Description: XMLUnit extends JUnit to simplify unit testing of XML. It compares a control
XML document to a test document or the result of a transformation, validates
documents against a DTD, and (from v0.5) compares the results of XPath
expressions.

Comment 1 Anthony Green 2007-03-07 16:40:03 UTC
This package doesn't build for me on FC6.  I don't know about rawhide.

docs:
    [mkdir] Created dir: /usr/src/redhat/BUILD/xmlunit/doc
  [javadoc] Generating Javadoc
  [javadoc] Javadoc execution
  [javadoc] java.lang.RuntimeException: Cannot read file specified in option
-overview: /usr/src/redhat/BUILD/xmlunit/src/java/overview.html (No such file or
directory)
  [javadoc]    at
gnu.classpath.tools.gjdoc.Main$20.process(gnu-classpath-tools-gjdoc-0.7.7.jar.so)
  [javadoc]    at
gnu.classpath.tools.gjdoc.Main.readOptions(gnu-classpath-tools-gjdoc-0.7.7.jar.so)
  [javadoc]    at
gnu.classpath.tools.gjdoc.Main.start(gnu-classpath-tools-gjdoc-0.7.7.jar.so)
  [javadoc]    at
gnu.classpath.tools.gjdoc.Main.main(gnu-classpath-tools-gjdoc-0.7.7.jar.so)

The RPM fails to find the javadocs when it tries to bundle things up.


Comment 2 Permaine Cheung 2007-03-13 03:52:44 UTC
Fixed the javadoc issues, added missing BR, fix classpath and some other rpmlint
issues.
Please review, spec file and srpm at:
https://pcheung.108.redhat.com/files/documents/174/295/xmlunit.spec
https://pcheung.108.redhat.com/files/documents/174/296/xmlunit-1.0-4jpp.1.src.rpm

Comment 3 Matt Wringe 2007-03-20 18:47:17 UTC
MUST:
* package is named appropriately
 - match upstream tarball or project name
 - try to match previous incarnations in other distributions/packagers for
consistency
 - specfile should be %{name}.spec
 - non-numeric characters should only be used in Release (ie. cvs or
   something)
 - for non-numerics (pre-release, CVS snapshots, etc.), see
   http://fedoraproject.org/wiki/Packaging/NamingGuidelines#PackageRelease
 - if case sensitivity is requested by upstream or you feel it should be
   not just lowercase, do so; otherwise, use all lower case for the name
OK
* is it legal for Fedora to distribute this?
 - OSI-approved
 - not a kernel module
 - not shareware
 - is it covered by patents?
 - it *probably* shouldn't be an emulator
 - no binary firmware
OK
* license field matches the actual license.
OK
* license is open source-compatible.
 - use acronyms for licences where common
OK
* specfile name matches %{name}
OK
* verify source and patches (md5sum matches upstream, know what the patches do)
 - if upstream doesn't release source drops, put *clear* instructions on
   how to generate the the source drop; ie. 
  # svn export blah/tag blah
  # tar cjf blah-version-src.tar.bz2 blah
Ok, md5sums match
* skim the summary and description for typos, etc.
X The summary is a little unclear. It should probably be something more like
"Unit Testing framework for XML"
* correct buildroot
 - should be:
   %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
OK
* if %{?dist} is used, it should be in that form (note the ? and %
locations)
OK
* license text included in package and marked with %doc
OK
* keep old changelog entries; use judgement when removing (too old?
useless?)
OK
* packages meets FHS (http://www.pathname.com/fhs/)
OK, appears to put everything in the proper locations.
* rpmlint on <this package>.srpm gives no output
X rpmlint xmlunit-1.0-4jpp.1.src.rpm
W: xmlunit non-standard-group Development/Testing
W: xmlunit invalid-license BSD Style Software License

The "Software License" part of the license should be removed to get rid of the
rpmlint warning.
The group warning can be ignored

* changelog should be in the proper format
OK
* Packager tag should not be used
OK
* Vendor tag should not be used
OK
* Distribution tag should not be used
OK
* use License and not Copyright 
OK
* Summary tag should not end in a period
OK
* if possible, replace PreReq with Requires(pre) and/or Requires(post)
OK
* specfile is legible
 - this is largely subjective; use your judgement
OK, looks good to me
* package successfully compiles and builds on at least x86
X doesn't build
* BuildRequires are proper
 - builds in mock will flush out problems here
X doesn't build in mock
 - the following packages don't need to be listed in BuildRequires:
   bash
   bzip2
   coreutils
   cpio
   diffutils
   fedora-release (and/or redhat-release)
   gcc
   gcc-c++
   gzip
   make
   patch
   perl
   redhat-rpm-config
   rpm-build
   sed
   tar
   unzip
   which
OK
* summary should be a short and concise description of the package
X summary should probably be changed (see above)
* description expands upon summary (don't include installation
instructions)
OK
* make sure lines are <= 80 characters
* specfile written in American English
OK
* make a -doc sub-package if necessary
 - see
  
http://fedoraproject.org/wiki/Packaging/Guidelines#head-9bbfa57478f0460c6160947a6bf795249488182b
X should the pdf be part of a manual package?
* packages including libraries should exclude static libraries if possible
* don't use rpath
* config files should usually be marked with %config(noreplace)
OK, I do not believe this package has any config files
* GUI apps should contain .desktop files
OK, not a GUI app
* should the package contain a -devel sub-package?
OK, it doesn't need one
* use macros appropriately and consistently
 - ie. %{buildroot} and %{optflags} vs. $RPM_BUILD_ROOT and $RPM_OPT_FLAGS
OK
* don't use %makeinstall
OK
* locale data handling correct (find_lang)
 - if translations included, add BR: gettext and use %find_lang %{name} at the
   end of %install
OK
* consider using cp -p to preserve timestamps
OK
* split Requires(pre,post) into two separate lines
OK
* package should probably not be relocatable
OK
* package contains code
 - see http://fedoraproject.org/wiki/Packaging/Guidelines#CodeVsContent
 - in general, there should be no offensive content
OK
* package should own all directories and files
* there should be no %files duplicates
* file permissions should be okay; %defattrs should be present
OK
* %clean should be present
OK
* %doc files should not affect runtime
* if it is a web apps, it should be in /usr/share/%{name} and *not* /var/www
* verify the final provides and requires of the binary RPMs
* run rpmlint on the binary RPMs
X can't build package

SHOULD:
* package should include license text in the package and mark it with %doc
* package should build on i386
* package should build in mock

I will finish the review once the package can be built


Comment 4 Permaine Cheung 2007-03-20 19:10:21 UTC
(In reply to comment #3)
> X The summary is a little unclear. It should probably be something more like
> "Unit Testing framework for XML"
Done
> * rpmlint on <this package>.srpm gives no output
> X rpmlint xmlunit-1.0-4jpp.1.src.rpm
> W: xmlunit non-standard-group Development/Testing
> W: xmlunit invalid-license BSD Style Software License
> 
> The "Software License" part of the license should be removed to get rid of the
> rpmlint warning.
Done

> X doesn't build
> * BuildRequires are proper
>  - builds in mock will flush out problems here
> X doesn't build in mock

> * summary should be a short and concise description of the package
> X summary should probably be changed (see above)
> * description expands upon summary (don't include installation
> instructions)
Done
> X should the pdf be part of a manual package?
Since there's only 1 pdf file, it should be ok to stay in there.

> X can't build package
> 
> SHOULD:
> * package should include license text in the package and mark it with %doc
> * package should build on i386
> * package should build in mock
> 
> I will finish the review once the package can be built
> 
I think we need to wait for mock on to-fcjpp1 to sync up with the latest
java-1.5.0-gcj.

Spec file and SRPM at the same location.

Comment 5 Matt Wringe 2007-03-20 19:40:04 UTC
Ok, the package now builds in mock.

rpmlint xmlunit-1.0-4jpp.1.fc7.src.rpm
W: xmlunit non-standard-group Development/Testing

rpmlint xmlunit-1.0-4jpp.1.fc7.noarch.rpm
W: xmlunit non-standard-group Development/Testing

rpmlint xmlunit-javadoc-1.0-4jpp.1.fc7.noarch.rpm
W: xmlunit-javadoc non-standard-group Development/Documentation

The rpmlint group warnings can be ignored.

SHOULD:
* package should include license text in the package and mark it with %doc
OK
* package should build on i386
OK
* package should build in mock
OK

This package now looks good to me, APPROVED



Comment 6 Permaine Cheung 2007-03-20 19:40:37 UTC
New Package CVS Request
=======================
Package Name: xmlunit
Short Description: Unit Testing framework for XML
Owners: pcheung
Branches: devel


Comment 7 Matt Wringe 2007-07-06 21:02:02 UTC
Has this package been built into CVS yet?

Comment 8 Permaine Cheung 2007-07-06 21:27:45 UTC
Yes, it's built.

Comment 9 Matt Wringe 2007-07-09 16:03:22 UTC
Closing as RAWHIDE


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