Bug 185257 - Review Request: raidem - 2d top-down shoot'em up
Review Request: raidem - 2d top-down shoot'em up
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Wart
Fedora Package Reviews List
:
Depends On: 185215 185216
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-03-12 16:05 EST by Hans de Goede
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-03-15 15:57:26 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
updated spec with desktop stuff (3.26 KB, text/plain)
2006-03-13 10:49 EST, Hans de Goede
no flags Details
raidem desktop file (238 bytes, text/plain)
2006-03-13 10:53 EST, Hans de Goede
no flags Details
raidem icon file (2.83 KB, image/png)
2006-03-13 11:30 EST, Hans de Goede
no flags Details
New specfile which handles the upstream source as requested (3.15 KB, text/plain)
2006-03-15 08:23 EST, Hans de Goede
no flags Details

  None (edit)
Description Hans de Goede 2006-03-12 16:05:23 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

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 16:12:52 EST
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 01:00:26 EST
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 01:08:56 EST
I spazzed on setting the bug dependencies last time.  This should fix it right up.
Comment 4 Hans de Goede 2006-03-13 10:49:28 EST
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 10:53:54 EST
Created attachment 126045 [details]
raidem desktop file
Comment 6 Hans de Goede 2006-03-13 11:30:51 EST
Created attachment 126052 [details]
raidem icon file
Comment 7 Wart 2006-03-14 13:34:17 EST
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 13:55:35 EST
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 14:23:49 EST
(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 15:39:04 EST
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 02:31:29 EST
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 08:23:28 EST
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 10:05:24 EST
mustfix items fixed.  APPROVED
Comment 14 Hans de Goede 2006-03-15 15:57:26 EST
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.