Red Hat Bugzilla – Bug 185257
Review Request: raidem - 2d top-down shoot'em up
Last modified: 2007-11-30 17:11:27 EST
Spec Name or Url: http://home.zonnet.nl/jwrdegoede/raidem.spec
SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/raidem-patches.tar.gz
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
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
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 :)
What do you think about my prboom comments?
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. :)
I spazzed on setting the bug dependencies last time. This should fix it right up.
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 :(
Created attachment 126045 [details]
raidem desktop file
Created attachment 126052 [details]
raidem icon file
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.
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.
(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.
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.)
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.
* 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
* 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.
* Delete the duplicate copies of the various libs during %prep, not by
modifying the upstream tarball.
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.
mustfix items fixed. APPROVED
imported and build, as usual many thanks for the review.