Bug 192060 - Review Request: blobwars - Mission and Objective based 2D Platform Game
Review Request: blobwars - Mission and Objective based 2D Platform Game
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Stone
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-17 05:33 EDT by Hans de Goede
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-06-08 02:16:41 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2006-05-17 05:33:13 EDT
Spec URL: http://people.atrpms.net/~hdegoede/blobwars.spec
SRPM URL: http://people.atrpms.net/~hdegoede/blobwars-1.05-1.src.rpm
Description:
Blob Wars : Metal Blob Solid. This is Episode I of the Blob Wars Saga.
You must undertake the role of fearless Blob solider, Bob, as he infiltrates
various enemy installations and hideouts in an attempt to rescue as many
MIAs as possible.
Comment 1 Jochen Schmitt 2006-05-17 14:17:16 EDT
Good:
* Tar in source RPM is the same as upstream
* Local build work fine
* Mock build worde fine.

Bad:
- Duplicate BuildRequires: SDL-devel (by SDL_mixer-devel)
Comment 2 Wart 2006-05-17 16:57:09 EDT
"This is Episode I of the Blob Wars Saga."

So where will I find Episode II?  ;)

There is a stray "export" in the %build section.  Leftover from debugging perhaps?
Comment 3 Hans de Goede 2006-05-17 17:08:35 EDT
(In reply to comment #1)
> 
> Bad:
> - Duplicate BuildRequires: SDL-devel (by SDL_mixer-devel)

Your right, but thats a minor issue, I'll fix it after import, or in a second
release / attempt if more issue are found during a proper review.

(In reply to comment #2)
> "This is Episode I of the Blob Wars Saga."
> 
> So where will I find Episode II?  ;)
> 

Episode II isn't finished yet but its undergoing active development and
a functional demo is downloadable:
http://www.parallelrealities.co.uk/blobAndConquer.php

> There is a stray "export" in the %build section.  Leftover from debugging perhaps?

My bad, it had an export CFLAGS=.... then I thought I could just as well pass
that directly to make, so I cut and pasted it to the make line and appearantly
forgot to remove the export.
Comment 4 Joost Soeterbroek 2006-05-30 16:28:40 EDT
Review for release 1:
* RPM name is OK
* Source blobwars_1.05.orig.tar.gz is the same as upstream
* Builds fine in mock
* rpmlint looks OK
* File list looks OK
* Works fine, including sound
Comment 5 Christopher Stone 2006-05-30 19:23:42 EDT
* rpmlint output clean
* Package named according to Package Naming Guidelines
* spec file name matches package %{name}
* Package meets Packaging Guidelines
* Licensed with open source compatible license
* License field in spec file matches actual license
* License text included in %doc
* spec file written in American English
* spec file is legible
* Sources match upstream
72b259abb6ec83a77f3c76052de24e50  blobwars_1.05.orig.tar.gz
* Package successfully compiles and builds on x86_64 FC-5
* All dependencies are specified in BuildRequires

- SDL-devel and ImageMagick are extraneous BR

* Spec file does not include any locales
* Package does not include any shared libraries
* Package is not relocatable
* Package owns all directories it creates
* Package does not contain duplicate files in %files
* File permissions are set appropriately
* Package contains a proper %clean section
* Macro usage is consistant
* Package contains permissible content
* Package does not contain large documentation to warrent a -doc subpackage
* Package does not contain any header files, static libraries or .pc files
* Package does not include any library files needed for a -devel subpackage
* Package does not contain any .la files
* Package includes a proper .desktop file
* Package does not own any files or directories owned by other packages


=== MUST FIX ====
* MUST remove extreanous BR SDL-devel and ImageMagick
* MUST remove extreanous "export" in %build section
* MUST remove extreanous "-n %{name}-%{version}" under %setup

=== SHOULD ===
* Please explain to me why you use patch -z, most examples I've seen use patch
-b and I don't understand the differences between these two options.  I'm not
asking you to use -b, but just explain to me why you use -z.  Thanks!
Comment 6 Christopher Stone 2006-05-30 19:46:18 EDT
Additional comment (SHOULD FIX):

/var/tmp/blobwars-1.05-1-root-mockbuild/usr/share/applications/fedora-blobwars.desktop:
warning: boolean key "Terminal" has value "0", boolean values should be "false"
or "true", although "0" and "1" are allowed in this field for backwards
compatibility
Comment 7 Christopher Stone 2006-05-30 19:48:26 EDT
NOTE SHOULD FIX:  If you are planning on fixing the .desktop file, I will also
recommend you call the game "Blob Wars: Metal Blob Solid" instead of just "Metal
Blob Solid".  This will make it easier to locate and will be easier when Part II
is released.
Comment 8 Hans de Goede 2006-06-01 06:27:15 EDT
(In reply to comment #5)
> === SHOULD ===
> * Please explain to me why you use patch -z, most examples I've seen use patch
> -b and I don't understand the differences between these two options.  I'm not
> asking you to use -b, but just explain to me why you use -z.  Thanks!

There is no difference, the syntax when calling the real patch is
patch -pX -b -z .suffix

Where -b forces backups and -z .suffix sets the suffix, hence I use -z .suffix
as that comes closes to the real patch.
%patch has either of:
%patch -pX -z .suffix
%patch -pX -b .suffix

And adss the -b before the -z or the -z between the -b and the .suffix (sigh).
So %patch is a bit weird.

I hope that explains. Here is a new version with fixes all MUST fix and should
fix items:
Spec URL: http://people.atrpms.net/~hdegoede/blobwars.spec
SRPM URL: http://people.atrpms.net/~hdegoede/blobwars-1.05-2.src.rpm
Comment 9 Hans de Goede 2006-06-07 05:04:30 EDT
I don't want to be rude, but ping?
Comment 10 Christopher Stone 2006-06-07 14:04:06 EDT
What more do you need me to do?  I approved this package a week ago...
Comment 11 Hans de Goede 2006-06-07 14:16:42 EDT
Oops, sorry

It seems that I don't get bugzilla mails when only the blocker bug changes (I'll
change my bugzilla mail settings to fix this). I'm kinda used to people adding a
comment when the approve.

Apologies!

Comment 12 Hans de Goede 2006-06-08 02:16:41 EDT
Imported and finally build (buildsys troubles), closing.

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