Bug 457277

Summary: Review Request: pentaho-reporting-flow-engine - Reporting Engine
Product: [Fedora] Fedora Reporter: Caolan McNamara <caolanm>
Component: Package ReviewAssignee: Jason Tibbitts <tibbs>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, tibbs, walters
Target Milestone: ---Flags: tibbs: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-09-07 17:07:37 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 457263    
Bug Blocks: 457945    

Description Caolan McNamara 2008-07-30 12:21:57 EDT
Spec URL: http://people.redhat.com/caolanm/jfreereport/pentaho-reporting-flow-engine.spec
SRPM URL: http://people.redhat.com/caolanm/jfreereport/pentaho-reporting-flow-engine-0.9.3-1.fc9.src.rpm
Description: A flow reporting engine, a dependency of OpenOffice.org3
Comment 1 Colin Walters 2008-08-06 10:05:09 EDT
Took a brief look; need to do the build deps first though.

[+]	source files match upstream
[+]	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.
 (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) is the
recommended value, but not the only one)
[+]	license field matches the actual license.
[+]	license is open source-compatible.
[+]	license text included in package.
[+]	latest version is being packaged.
[+]	BuildRequires are proper
[+]	%clean is present.

-- TO BE REVIEWED

[?]	package builds in koji
[?]	package installs properly
[?]	rpmlint is silent.
please fix.
[?]	owns the directories it creates.
[?]	doesn't own any directories it shouldn't.
[?]	no duplicates in %files.
[?]	file permissions are appropriate.
[?]	code, not content.
[?]	documentation is small, so no -docs subpackage is necessary.
[?]	%docs are not necessary for the proper functioning of the package.
Comment 2 Jason Tibbitts 2008-09-05 00:05:07 EDT
Since I just did a favorable sort-of review of the last remaining build dep (liblayout), I stuck it in a local repo and tried to build this.  It builds fine but won't install because liblayout >= 0.3.7 is required.  This is a bit confusing to me, because the version of liblayout that was submitted is 0.2.9, and this seems to be the latest upstream version.  Where is 0.3.7 supposed to come from?
Comment 4 Jason Tibbitts 2008-09-05 10:26:13 EDT
OK, builds and installs fine now.  rpmlint says:

  pentaho-reporting-flow-engine.src:87: W: libdir-macro-in-noarch-package 
   %attr(-,root,root) %{_libdir}/gcj/%{name}
which comes from rpmlint not parsing the conditional arch-ness of this package.

  pentaho-reporting-flow-engine-javadoc.x86_64: W: non-standard-group
   Development/Documentation
which is OK because we don't care what goes in group.  And:

  pentaho-reporting-flow-engine-debuginfo.x86_64: E: description-line-too-long 
   This package provides debug information for package 
   pentaho-reporting-flow-engine.
which could use a line wrap.

I'll go ahead and apprrove this and you can tweak that description when you check in.

* source files match upstream:
   9ddd78ea9239cefbad21e81621bdc4f2c1f1a9a42a247d38100d49882d90d475  
   pentaho-reporting-flow-engine-0.9.3.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK (javadoc description needs a line wrap).
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
X rpmlint has one valid, but minor complaint.
* final provides and requires are sane:
  pentaho-reporting-flow-engine-0.9.3-2.fc10.x86_64.rpm
   pentaho-reporting-flow-engine.jar.so()(64bit)
   pentaho-reporting-flow-engine = 0.9.3-2.fc10
   pentaho-reporting-flow-engine(x86-64) = 0.9.3-2.fc10
  =
   /bin/sh
   flute
   java
   java-gcj-compat >= 1.0.31
   jcommon
   jcommon-serializer
   jpackage-utils
   libfonts >= 0.3.4
   libformula >= 0.1.18
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libgcj_bc.so.1()(64bit)
   liblayout >= 0.2.9
   librepository >= 0.1.6
   libz.so.1()(64bit)
   pentaho-libxml
   sac

  pentaho-reporting-flow-engine-javadoc-0.9.3-2.fc10.x86_64.rpm
   pentaho-reporting-flow-engine-javadoc = 0.9.3-2.fc10
   pentaho-reporting-flow-engine-javadoc(x86-64) = 0.9.3-2.fc10
  =
   jpackage-utils
   pentaho-reporting-flow-engine = 0.9.3-2.fc10

* %check is not present; no test suite.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (rebuild-gcj-db).
* no pre-built jars
* single jar, named after the package
* jarfiles are under _javadir.
* javadocs are under _javadocdir.
* ant called properly.
* gcj called properly.

APPROVED, just line-wrap the javadoc description at your leisure.
Comment 5 Caolan McNamara 2008-09-05 10:51:54 EDT
New Package CVS Request
=======================
Package Name: pentaho-reporting-flow-engine
Short Description: A flow reporting engine
Owners: caolanm
Branches:
InitialCC:

Thanks tibbs, this is the last package in a chain which has taken nearly 6 months from the first review to the last, and I appreciate being able to see the back of it.
Comment 6 Jason Tibbitts 2008-09-05 11:08:33 EDT
Sorry it took so long.  Once the java guidelines were done, I had intended to run through all of them but I stopped at the point where others had assigned themselves to the tickets.  I hope that whatever you needed these packages for doesn't miss the freeze.
Comment 7 Kevin Fenzi 2008-09-05 12:56:34 EDT
cvs done.