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

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 David Timms <dtimms(a)iinet.net.au&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dtimms(a)iinet.net.au Blocks|2 |3 AssignedTo|rpmfusion-package- |dtimms(a)iinet.net.au |review(a)rpmfusion.org | --- Comment #1 from David Timms <dtimms(a)iinet.net.au&gt; 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.