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 192060

Summary: Review Request: blobwars - Mission and Objective based 2D Platform Game
Product: [Fedora] Fedora Reporter: Hans de Goede <hdegoede>
Component: Package ReviewAssignee: Christopher Stone <chris.stone>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: joost.soeterbroek, wart
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-06-08 06:16:41 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    

Description Hans de Goede 2006-05-17 09:33:13 UTC
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 18:17:16 UTC
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 20:57:09 UTC
"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 21:08:35 UTC
(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 20:28:40 UTC
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 23:23:42 UTC
* 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 23:46:18 UTC
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 23:48:26 UTC
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 10:27:15 UTC
(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 09:04:30 UTC
I don't want to be rude, but ping?


Comment 10 Christopher Stone 2006-06-07 18:04:06 UTC
What more do you need me to do?  I approved this package a week ago...

Comment 11 Hans de Goede 2006-06-07 18:16:42 UTC
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 06:16:41 UTC
Imported and finally build (buildsys troubles), closing.