Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 185257 - Review Request: raidem - 2d top-down shoot'em up
Summary: Review Request: raidem - 2d top-down shoot'em up
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Wart
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 185215 185216
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-03-12 21:05 UTC by Hans de Goede
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-03-15 20:57:26 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
updated spec with desktop stuff (deleted)
2006-03-13 15:49 UTC, Hans de Goede
no flags Details
raidem desktop file (deleted)
2006-03-13 15:53 UTC, Hans de Goede
no flags Details
raidem icon file (deleted)
2006-03-13 16:30 UTC, Hans de Goede
no flags Details
New specfile which handles the upstream source as requested (deleted)
2006-03-15 13:23 UTC, Hans de Goede
no flags Details

Description Hans de Goede 2006-03-12 21:05:23 UTC
Spec Name or Url: http://home.zonnet.nl/jwrdegoede/raidem.spec
SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/raidem-patches.tar.gz

Description:
Raid'em is a 2d top-down shoot'em up. It began as a remake of Raid II   
(abandoned long ago), which was inspired by Raptor, but has turned out very
differently. Features: Neat looking graphics, LOTS of explosions and scrap
metal, Eye candy a-plenty, Many different powerups, A desert. And a space
platform. And some snow, 2 player mode, Demo recording and playback, Loads of
fun.

Sorry, no SRPM to big (6Mb) for my ISP, the patches are in the tgz. See the spec for howto _generate_ the needed main-src.tar.gz

Comment 1 Hans de Goede 2006-03-12 21:12:52 UTC
Michael,

Me again this is the game for which I've been packaging adime and glyph-keeper.
I know I still need to add a .desktop file, but its late over here now :)

p.s.

What do you think about my prboom comments?


Comment 2 Wart 2006-03-13 06:00:26 UTC
I made this bug dependent on glyph-keeper and adime.  I'll try to get to it
after I'm done reviewing glyph-keeper.  I'd have done this sooner but I've been
extensively "testing" prboom + freedoom today.  :)

Comment 3 Wart 2006-03-13 06:08:56 UTC
I spazzed on setting the bug dependencies last time.  This should fix it right up.

Comment 4 Hans de Goede 2006-03-13 15:49:28 UTC
Created attachment 126043 [details]
updated spec with desktop stuff

Hmm, this is becoming a mess, sorry. I really should get a decent ISP. I'm
moving at the end of this year, which will be a good moment for this.

Anyways attached is a new spec and I'll also add the .desktop and .png (ico)
file. As already explained I can't upload a srpm as my isp doesn't allow access
from anywhere but home and also it would be to big :(

Comment 5 Hans de Goede 2006-03-13 15:53:54 UTC
Created attachment 126045 [details]
raidem desktop file

Comment 6 Hans de Goede 2006-03-13 16:30:51 UTC
Created attachment 126052 [details]
raidem icon file

Comment 7 Wart 2006-03-14 18:34:17 UTC
Instead of removing the duplicate libraries in the source (glyph-keeper, adime,
etc.), the building of these libraries should just be disabled in the Makefile.
 This will help keep the source as close of a match to upstream as possible. 
The annoying -src in the tarball should remain as well.

The mp3 encoder should still be rm'd, however.

The license should also be "zlib License".

I'll try to do a formal review later today.

Comment 8 Hans de Goede 2006-03-14 18:55:35 UTC
Unfortunatly its not that easy, not all libraries included can be disabled in
the Makefile and because loadpng from the lib dir is actually needed -Ilib must
be added, thus leaving these directories under lib can cause the header file
under lib to be used instead of the system installed version .

I could leave them in and rm them under %prep if you think thats preferable.

About the license, thats already fixed in the attached updated version of the
.spec with the desktopfile stuff added to it.


Comment 9 Wart 2006-03-14 19:23:49 UTC
(In reply to comment #8)
> Unfortunatly its not that easy, not all libraries included can be disabled in
> the Makefile and because loadpng from the lib dir is actually needed -Ilib must
> be added, thus leaving these directories under lib can cause the header file
> under lib to be used instead of the system installed version .
> 
> I could leave them in and rm them under %prep if you think thats preferable.

Yes, I think that would be better.

> About the license, thats already fixed in the attached updated version of the
> .spec with the desktopfile stuff added to it.

My mistake.  I was looking at the wrong spec file.

Comment 10 Hans de Goede 2006-03-14 20:39:04 UTC
Kind request, can you do the full review with the sources as is (iow using a
soruce tarbell with the -src and lib dir stripped), then I can fix all problems
in one go (I hope.)

Thanks!


Comment 11 Wart 2006-03-15 07:31:29 UTC
rpmlint warnings:

W: raidem invalid-license zlib License
W: raidem-debuginfo invalid-license zlib License

These can be ignored.  This is an acceptible license.

W: raidem uncompressed-zip /usr/share/raidem/data/wraith.zip
W: raidem uncompressed-zip /usr/share/raidem/data/proj-maser.zip
W: raidem uncompressed-zip /usr/share/raidem/data/sea-urchin.zip
W: raidem uncompressed-zip /usr/share/raidem/data/conquerer.zip
W: raidem uncompressed-zip /usr/share/raidem/data/spirtle.zip
W: raidem uncompressed-zip /usr/share/raidem/data/zippy.zip
W: raidem uncompressed-zip /usr/share/raidem/data/bolt.zip
W: raidem uncompressed-zip /usr/share/raidem/data/schemer.zip
W: raidem uncompressed-zip /usr/share/raidem/data/proj-pulselaser.zip

These can also be ignored.  These files contain game data that are
extracted by the game engine at runtime.

MUST
====

* Package and spec named appropriately
* License 'zlib' ok, license file included
* spec file legible and in Am. English
* Builds and runs on FC-5 i386
* No excessive or inappropriate BR: or Requires:
* No locales
* No shared libraries
* Not relocatable
* Owns all directories that it creates
* No duplicates %files
* Permissions look ok
* No -devel package necessary
* Contains both code and permissible content (game data)
* Macro use is consistent and sane
* buildroot cleaned during %install and %clean
* No .la archives or static libs
* desktop file included, looks ok

NON-BLOCKING
============
* Source does not match upstream, for a good reason.  Upstream source
  contains a mp3 encoder that can't be included in FE.  The game still
  functions without it, so the offending code has been removed.

MUSTFIX
=======
* Delete the duplicate copies of the various libs during %prep, not by
  modifying the upstream tarball.


Comment 12 Hans de Goede 2006-03-15 13:23:28 UTC
Created attachment 126154 [details]
New specfile which handles the upstream source as requested

Thanks for the review, I've attached a new spec which leaves the upstream
source as intact as possible as requested.

Comment 13 Wart 2006-03-15 15:05:24 UTC
mustfix items fixed.  APPROVED

Comment 14 Hans de Goede 2006-03-15 20:57:26 UTC
imported and build, as usual many thanks for the review.



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