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 (3.26 KB, text/plain)
2006-03-13 15:49 UTC, Hans de Goede
no flags Details
raidem desktop file (238 bytes, text/plain)
2006-03-13 15:53 UTC, Hans de Goede
no flags Details
raidem icon file (2.83 KB, image/png)
2006-03-13 16:30 UTC, Hans de Goede
no flags Details
New specfile which handles the upstream source as requested (3.15 KB, text/plain)
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.