[Bug 302361] Review Request: freecol - The FreeCol multi-player strategy game

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: freecol - The FreeCol multi-player strategy game


https://bugzilla.redhat.com/show_bug.cgi?id=302361





------- Additional Comments From j.w.r.degoede at hhs.nl  2007-09-30 14:33 EST -------
(In reply to comment #1)
> Initial notes, not yet a full review:
> 
> Doesn't Java 1.7 include the StAX APIs and a reference implementation?  That
> would probably make all dependencies on wstx superfluous.  In fact, the package
> builds fine with the dependencies removed and no wstx available.  I don't have a
> setup where this could be tested at runtime, but at least build without wstx
> succeeds.
> 

I've indeed tested without wstx and it works, but the reference inplementation
is SLOW. When clicking start new game, without wstx it takes 30 seconds for the
game to actually start, with wstx only circa 4 seconds.

I could do a version without wstx for now and then add wstx once it gets
approved, or we could work together to get wstx approved (one as submitter one
as reviewer, making the current submitter co-maintainer).

> Hardwired Class-Path entries in jar manifests are generally frowned upon, and in
> this case they point to stuff in the jars/ subdir which won't exist as
> freecol.jar is installed to /usr/share/java and the startup script takes care of
> the classpath.  So they should be removed.
> 

Agreed, will fix.

> The Encoding key in the desktop entry is deprecated, and could be removed.
> 

I didn't know, this is the first time anyone has told me this, is this
documented somewhere?

> rpmlint:
> freecol-manual.noarch: E: zero-length /usr/share/doc/freecol-manual-0.7.2/img2.png
> freecol-manual.noarch: E: zero-length /usr/share/doc/freecol-manual-0.7.2/img1.png
> freecol-manual.noarch: E: zero-length /usr/share/doc/freecol-manual-0.7.2/img3.png
> 
> Intentional?

[hans at shalem freecol]$ ls -l /usr/share/doc/freecol-manual-0.7.2/
total 6304
-rw-r--r-- 1 root root     891 2007-09-23 22:06 FreeCol.css
-rw-r--r-- 1 root root    6035 2007-09-23 22:06 FreeCol.html
-rw-r--r-- 1 root root 3230763 2007-09-23 22:06 FreeCol.pdf
-rw-r--r-- 1 root root  687526 2007-09-23 22:06 colony_panel.png
-rw-r--r-- 1 root root  680906 2007-09-23 22:06 europe_panel.png
-rw-r--r-- 1 root root     362 2007-09-23 22:06 img1.png
-rw-r--r-- 1 root root     392 2007-09-23 22:06 img2.png
-rw-r--r-- 1 root root     188 2007-09-23 22:06 img3.png
-rw-r--r-- 1 root root    6035 2007-09-23 22:06 index.html

<snip>

Looks like a bug in your version of "javadoc" to me.

---

(In reply to comment #2)
> Created an attachment (id=211381)
 --> (https://bugzilla.redhat.com/attachment.cgi?id=211381&action=view) [edit]
> Suggested specfile changes
> 
> This patch gets rid of the wstx dependency as well as the manifest Class-Path
> entry.	Additionally, if the wstx dep can go indeed, it should be removed from
> the startup script too.

Thanks for the patch for the ClassPath removal, as for wstx, see above.


-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.