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 194436 - Review Request: wormux - 2D Kill 'em all game
Summary: Review Request: wormux - 2D Kill 'em all game
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Stone
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-06-08 06:01 UTC by Wart
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-06-14 20:05:26 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Wart 2006-06-08 06:01:23 UTC
Spec URL: http://www.kobold.org/~wart/fedora/wormux.spec
SRPM URL: http://www.kobold.org/~wart/fedora/wormux-0.7.2-1.src.rpm
Description: 2D kill 'em all game in the same vein as Scorched Earth

Comment 1 Wart 2006-06-14 20:05:26 UTC
[Reposting review comments due to bugzilla data loss]
------- Additional Comments From hugo.br  2006-06-09 16:09 EST -------
My Review:

OKS

 * Package's name is ok
 * rpmlint returns ok
 * Spec name is ok and legible in English
 * License is ok and included in %doc
 * Package's source matches upstream checksum
 * Package builds fine
 * BuildRequires are ok
 * Package's %files are ok and all directories owned
 * Package's %clean is ok
 * Use of macros and scriptlets are fine

SHOULD

 * Currently the package doesn't have locale files and the %find_lang macro 
isn't necessary. But in the future this could change, so I think it should be 
used from now
 * You should create a sub-package to the data files, for example, 
wormux-data. This will save the users' bandwidth when minor fixes are applied 
to the main package.
 * The description field is too vague. Try describing the game better  ;-) 

------- Additional Comments From wart  2006-06-09 17:09 EST -------
(In reply to comment #1)


>> SHOULD
>> 
>>  * Currently the package doesn't have locale files and the %find_lang macro 
>> isn't necessary. But in the future this could change, so I think it should be 
>> used from now


I prefer not to include %find_lang until there are actual lang files.  Otherwise
it leads to extra misleading cruft in the spec file.


>>  * You should create a sub-package to the data files, for example, 
>> wormux-data. This will save the users' bandwidth when minor fixes are applied 
>> to the main package.


Good point.  Done.  Note that this won't actually reduce the size of the
downloads, since a new -data subpackage will automatically get built when the
game engine is updated.  upstream should package the game data in a separate
tarball for us to really benefit from the -data subpackage.


>>  * The description field is too vague. Try describing the game better  ;-) 


Yeah, I kinda flaked on that.  I tend to cut and paste the descriptions from the
packages' home pages.  In this case, the home page didn't have a decent
description when I first wrote the spec.  It should look better now.

http://www.kobold.org/~wart/fedora/wormux-0.7.2-2.src.rpm
http://www.kobold.org/~wart/fedora/wormux.spec

------- Additional Comments From chris.stone  2006-06-09 18:06 EST -------
* rpmlint output okay:
W: wormux-data no-documentation
* Package is named according to package naming guidelines
* spec file matches package %{name}
* Package meets packaging guidelines
* Package licensed with open source compatible license
* License in spec file matches actual license
* License text file included in %doc
* Spec file written in American english
* Spec file is legible
* Sources match upstream source
08d897a89f06cb855709be2904308cac  wormux-0.7.2.tar.gz
* Package successfully compiles and builds on x86_64 FC-5
* All build dependencies are listed in BuildRequires

- Except for pkgconfig which will be needed for FC-6

* No locales in package
* Package does not make a shared library
* Package is not relocatable
* Package owns all directories it creates
* Package does not contain duplicate files in %files
* Permissions on files set properly
* Package has appropriate %clean section

- Macro usage is not consistant

* Package contains permissible content
* Package does not contain large documentation to warrent a -doc subpackage
* Files in %doc do not affect runtime of application
* Package does not contain header files or static libraries
* Package does not contain any .pc files
* Package does not contain any .so files
* Package does not need a devel subpackage
* Package does not contain any .la files
* Package contains a nearly appropriate .desktop file

- .desktop file missing Encoding section

* Package does not own files or directories owned by other packages


MUST:
- Add pkgconfig to BuildRequires for FC6 builds
- Add "Encoding" field to .desktop file
- Use %{buildroot} consistantly, there is an $RPM_BUILD_ROOT adn %{buildroot}
- Remove INSTALL from %files

Notes:
- Credits button doesn't seem to do anything, not sure if its broken or just not
implemented yet.

------- Additional Comments From wart  2006-06-09 18:44 EST -------
(In reply to comment #3)

>> MUST:
>> - Add pkgconfig to BuildRequires for FC6 builds


This shouldn't be necessary.  There is already a BR: libxml++-devel which itself
requires pkgconfig
 

>> - Add "Encoding" field to .desktop file


Fixed


>> - Use %{buildroot} consistantly, there is an $RPM_BUILD_ROOT adn %{buildroot}


Fixed


>> - Remove INSTALL from %files


Fixed


>> Notes:
>> - Credits button doesn't seem to do anything, not sure if its broken or just not
>> implemented yet.


I'll take a closer look at that.

Here's a new package that fixes all of the above except for the broken credits
button:

http://www.kobold.org/~wart/fedora/wormux-0.7.2-3.src.rpm
http://www.kobold.org/~wart/fedora/wormux.spec

------- Additional Comments From chris.stone  2006-06-09 18:58 EST -------
** APPROVED **

Noticed that "exerminate" is missing a "t".  Non-blocker, but please fix.

------- Additional Comments From wart  2006-06-09 19:02 EST -------
I also discovered a bad icon path in the -3 release.  I'll fix that as well
before checking in.

Thanks for the review!

------- Additional Comments From wart  2006-06-09 22:50 EST -------
Imported and built on -devel




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