[Bug 509664] Review Request: tremfusion - Enhanced modification of the free software first person shooter Tremulous

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=509664


Peter Gordon <peter at thecodergeek.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |needinfo?(ian at ianweller.org
                   |                            |)




--- Comment #5 from Peter Gordon <peter at thecodergeek.com>  2009-07-05 15:14:04 EDT ---
The only other issue I see at first glance is that the some of the data (in
%{_datadir}/%{name}) could potentially be in its own noarch subpackage, for
less churn when updating, etc.; but that data is reasonably small enough (just
over a megabyte) that splitting the package even further just for this purpose
seems a bit overkill.  Oh well. 

Full review of tremfusion 0.99-3.r3 follows.


++ GOOD:
 * rpmlint against the binary packages is clean.
 * Naming and version/release are good. Spec file name is "%{name}.spec" as
required.
 * %changelog section is valid.
 * BuildRoot is properly defined, and cleaned both as the first step in
%install and as the only step in %clean.
 * Other than the server subpackage, the Licenses (mix of CC-BY-SA and GPLv2+)
are acceptable for Fedora and match the actual licenses (as mentioned in the
README file), copies of which are properly included in the %doc listings.
 * Spec file is in American English, and is legible.
 * Successfully builds in mock on both Rawhide and F-11 (tested on x86_64).
 * It installs and runs just fine (which was why I waited until today to do the
review, sorry... XD)
 * Timestamps are kept ("install -p")
 * Sources match those of upstream. (Not a pristine tarball, so I only checked
that the included method of acquiring the sources produced a tarball whose file
listings matched those provided in the linked SRPM.)
 * Ownership and permissions of files/directories are sane with no duplicates
in the %files listing; and the %defattr line is good. 
 * Final Requires/Provides list is sane.
 * Macro variable usage is consistent.
 * Builds/runs agains the system libraries instead of local copies (except for
the libjpeg difference, as you noted).
 * Files marked as %doc are not required at runtime.
 * Dependencies OK between subpackages and the main package.
 * No libtool (.la) files present in built package.
 * All filenames in built package are valid UTF-8.
 * Package contains no translations (so %find_lang stuff is not necessary).
 * Package contains no static libraries, header files, or pkconfig data.
 + Game data (tremulous-data, required by tremfusion-common) is separate from
the content/binaries.
 + License files included as part of %doc.

++ BAD:

 (1) The server subpackage needs a License tag.
 (2) The server should run as its own user, not as 'root' or 'games' or any
other system user account. (See
http://fedoraproject.org/wiki/Packaging/UsersAndGroups for detailed
instructions).
 (3) You should also use the opengl-games-utils wrapper, as indicated on the
Games SIG wiki page:
http://fedoraproject.org/wiki/SIGs/Games/Packaging#OpenGL_Wrapper .

Fix up those three issues, and it'll garner my approval. :)

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.