Bug 534135 - Review Request: xstream - Java XML serialization library
Review Request: xstream - Java XML serialization library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Michal Ingeli
Fedora Extras Quality Assurance
F13FeatureIDEA
:
Depends On: 469894
Blocks: 534168
  Show dependency treegraph
 
Reported: 2009-11-10 12:19 EST by Lubomir Rintel
Modified: 2009-12-03 04:56 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-12-03 04:56:38 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mi: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
Build log (28.02 KB, text/plain)
2009-11-23 13:23 EST, Jochen Schmitt
no flags Details

  None (edit)
Description Lubomir Rintel 2009-11-10 12:19:54 EST
SPEC: http://v3.sk/~lkundrak/SPECS/xstream.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/xstream-1.2.2-2.src.rpm

Description:

XStream is a simple library to serialize objects to XML 
and back again. A high level facade is supplied that 
simplifies common use cases. Custom objects can be serialized 
without need for specifying mappings. Speed and low memory 
footprint are a crucial part of the design, making it suitable 
for large object graphs or systems with high message throughput. 
No information is duplicated that can be obtained via reflection. 
This results in XML that is easier to read for humans and more 
compact than native Java serialization. XStream serializes internal 
fields, including private and final. Supports non-public and inner 
classes. Classes are not required to have default constructor. 
Duplicate references encountered in the object-model will be 
maintained. Supports circular references. By implementing an 
interface, XStream can serialize directly to/from any tree 
structure (not just XML). Strategies can be registered allowing 
customization of how particular types are represented as XML. 
When an exception occurs due to malformed XML, detailed diagnostics 
are provided to help isolate and fix the problem.
Comment 1 Lubomir Rintel 2009-11-10 12:22:52 EST
RPMLint:

xstream.src:198: W: libdir-macro-in-noarch-package (main package) %dir %{_libdir}/gcj/%{name}
xstream.src:199: W: libdir-macro-in-noarch-package (main package) %{_libdir}/gcj/%{name}/%{name}-*%{version}.jar.*

False alarms, rpmlint does not grok %if-s.

xstream.i686: W: wrong-file-end-of-line-encoding /usr/share/doc/xstream-1.2.2/LICENSE.txt

Some packagers prefer not to touch license files and I am one of them.

Note to reviewer:

I made this depend on bug #469894 which is a dead review. I hope it come back to life, if not I'll open a new one. However, to review this a package you can get there (cglib) is needed.
Comment 2 Jochen Schmitt 2009-11-15 15:26:58 EST
Question: Do you really want the gcj stuff. This will produced an architecture depending package.
Comment 3 Lubomir Rintel 2009-11-20 10:44:07 EST
Jochen, it's conditionally disabled and in fact still used on ppc. I personally don't add it to new packages, but when it's already there I'm reluctant to remove it since it still might be useful for some (powerpc). More on topic:

https://www.redhat.com/archives/fedora-devel-java-list/2009-November/msg00024.html
Comment 4 Jochen Schmitt 2009-11-23 13:23:18 EST
Created attachment 373195 [details]
Build log

Failed build log on mock agains fedora-development-x86_64
Comment 5 Lubomir Rintel 2009-11-27 06:57:38 EST
Jochen: Changed to build with OpenJDK, and since we don't build with gcj, AOT was removed as well.

SPEC: http://v3.sk/~lkundrak/SPECS/xstream.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/xstream-1.2.2-3.src.rpm
Comment 7 Michal Ingeli 2009-11-27 07:59:58 EST
- builds in koji
- license ok

* rpmlint not silent:
  - xstream.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/xstream-1.2.2/LICENSE.txt
    I also think, that it's not good to touch license, so this is OK.
  - xstream.noarch: W: incoherent-version-in-changelog 0:1.2.2-3 ['1.2.2-3', '1.2.2-3']
    Remove epoch also from changelog

Dropping that AOT seems OK, according to [1] it will be no longer required in F13.

APPROVED

[1] https://www.redhat.com/archives/fedora-devel-java-list/2009-November/msg00025.html
Comment 8 Lubomir Rintel 2009-11-27 09:37:02 EST
O kruwa, dziekuje.

New Package CVS Request
=======================
Package Name: xstream
Short Description: Java XML serialization library
Owners: lkundrak
Branches: F-11 F-12 EL-5
Comment 9 Jochen Schmitt 2009-11-30 15:32:49 EST
@Mical
Please assign this bug to you.

At least: from my point of view your review is very poor.

I'm waiting for the CVS import, because I want to do the review for groovy.
Comment 10 Michal Ingeli 2009-11-30 18:34:43 EST
(In reply to comment #9)
> @Mical
> Please assign this bug to you.

I already have.
 
> At least: from my point of view your review is very poor.

It's more "brief". I tried to build package locally, in mock and finally in koji, and it passed. Another important thing is license, and that's also ok. 

I gave it another check. There is missing one "must" from java guidelines, and that is "Require: java", jpackage-utils is not enough.

- correctly named
- source matches upstream
- license in %doc
- clean, legible, american english

* I think, javadoc shouldn't be in %doc, but there is not a guideline for that
  %doc %{_javadocdir}/%{name}-%{version}
* add java require (shame, that I missed this one!)
* drop epoch from current changelog entry

If you see anything, that is not fulfilling the guidelines, feel free to point it out.
Comment 11 Lubomir Rintel 2009-12-02 17:11:00 EST
(In reply to comment #10)
> (In reply to comment #9)
> > At least: from my point of view your review is very poor.
> 
> It's more "brief". I tried to build package locally, in mock and finally in
> koji, and it passed. Another important thing is license, and that's also ok.

Jochen, seriously, what was wrong here? Please point out any deficiencies of the package, I have a feeling that the CVS request might be delayed for this (other, older cvs requests have been processed), so I'd really like to have this cleared up.

> I gave it another check. There is missing one "must" from java guidelines, and
> that is "Require: java", jpackage-utils is not enough.

Fair enough. Fixed.

> * I think, javadoc shouldn't be in %doc

Fixed.

> * drop epoch from current changelog entry

Done.

New package:

SPEC: http://v3.sk/~lkundrak/SPECS/xstream.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/xstream-1.2.2-4.fc13.src.rpm
Comment 12 Kevin Fenzi 2009-12-03 01:30:48 EST
cvs done.
Comment 13 Lubomir Rintel 2009-12-03 04:56:38 EST
Imported and built.

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