[Bug 269] Review Request: snowballz - a fun RTS game featuring snowball fights with penguins

[RPM Fusion Bugzilla]

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269


David Timms <dtimms at iinet.net.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtimms at iinet.net.au
             Blocks|2                           |3
         AssignedTo|rpmfusion-package-          |dtimms at iinet.net.au
                   |review at rpmfusion.org        |




--- Comment #1 from David Timms <dtimms at iinet.net.au>  2009-01-05 13:17:16 ---
Review for snowballz:
=====
OK no problemo
NA not applicable
?? can't say/don't understand.
! needs work.
=====
[OK] rpmlint .src.rpm :
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[! ] rpmlint
/home/davidt/rpmbuild/RPMS/noarch/snowballz-1.0-0.1.beta1.20081222hg.fc10.noarch.rpm
:
snowballz.noarch: E: zero-length
/usr/share/games/snowballz/data/maps/crystalcrises/dynamic_objects.txt
snowballz.noarch: E: zero-length
/usr/share/games/snowballz/data/maps/world1/dynamic_objects.txt
snowballz.noarch: E: zero-length
/usr/share/games/snowballz/data/maps/td1/dynamic_objects.txt
snowballz.noarch: E: zero-length
/usr/share/games/snowballz/data/maps/crystalcrises/script_areas.txt
1 packages and 0 specfiles checked; 4 errors, 0 warnings.

Are these zero length files required for snowballz to work ?

[OK] included source is pulled from hg so direct md5sum compare isn't possible.
Since package includes a snowballz-snapshot script, this was used to retrieve
1.0beta1 from the mercurial repo, which created a source archive with today's
date. While these two archives differ slightly in size, I think that is to be
expected because the file dates within the archive are stored - and this
probably leads to different compression levels.

The freshly retrieved archive was extracted, and the included source rpm
archive also extracted. diff -ur between the two extracted trees shows no
differences.

[OK] package name is ok, it's the only user in a web search.
[OK] spec named %name.spec as required
[OK] source tarball contains no prebuilt binaries/libraries.
[OK] files placed into FHS locations - meets fedora python standard
[OK] changelog in standard format
[OK] correctly omits Packager, vendor, copyright, prereq 
    , includes license tag.
[OK] summary is <80 chars, no ending period
[OK] source0 is a direct archive made with the snapshot script that retrieves
it from mercurial repo.
[OK] buildroot set at second most preferred location:
[OK] %install correctly erases buildroot before build
[OK] mock build succeeds.
[OK] rpmdiff between default build and mock build shows only time
    {T} differences for folders and the pre-compiled .py[co] files
[OK] description is column limited to <80 chars.
[OK] man 6 is included.
[OK] charset is ascii
[OK] debuginfo not expected in noarch application.
[OK] no static libraries, rpath, self copies of already packaged libaries
    etc, since pure python project.
[OK] no config, initscripts, 
[OK] variable style is used consistently
[NA] not multilingual
[OK] timestamps are kept
[NA] make is python based, so smp_flags not required.
[OK] build requires python.
[OK] no conditional dependencies
[OK] is program code and game data (graphics, sounds)
[NA] no provides to consider
[OK] dirs/files owned by the package
[OK] not a web app, shouldn't conflict with other packages.
[NA] python sitelib applies only to python libraries
[OK] eggs are built from source into binary rpm.
  - dont know much about this, but the folder is created with various info
files inside it, so looks OK.

[OK] no files in %{_bindir} and %{_sbindir}.
[OK] %install setup.py install --skip-build. as guided for python packages.
[OK] %files: 
  - is it normal to stick games in ../games/%{name} ? (Well I see fortune and
torcs are, and frozen-bubble isn't so I guess I answered that.)
  - the icon, manual, and desktop entry are specifically noted, while the
../games/snowballz/ folder is defined, so this looks good to me.
[NA] there are no unit tests, that I can see.

--- issues:   
[! ] Summary:        A fun RTS game featuring snowball fights with penguins
  - It might be worth expanding RTS into what it means (for those that don't
know), 

[!] desktop files has been created. It uses a "beta" trailer in the title,
which is useful to inform players that there could be problems in the game.
  The groups meet freedesktop standard, as required.
  - GenericName=RTS Game
Maybe: Graphical Real Time Strategy Game 
  - Comment=Command your army of penguins
Maybe: A game where you command an army of penguins

[! ] license "GPLv2+ and MIT": the only licensing I can see is in:
snowui/__init__.py
but it isn't a well known license. Why did you select these as the license ?

[??] in setup, ttf fonts are removed. Would it be better to remove them
(TSCu_Comic.ttf) in the snapshot creation process, so that they don't make it
into the .src.rpm ?
    This might make it more difficult once there are release source archives.
    Is the reason to remove it because it's license is unclear ? A comment to
describe why would help readers.
    Do you need to (somehow) replace the font with font already in Fedora ? 

[??] no scriptlets
    why would you build the wrapper script during setup, rather than separately
storing the script in cvs ? 
    Would it be better to be able to keep the creation time from cvs ?

[! ] waiting on Requires: pyglet to be able to install built package and test
functionality.
====
Needs work, and or brief spec file comments for some choices youhave made, in
my opinion, and waiting on other reviews.


-- 
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
You are the assignee for the bug.