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@xxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(ian@xxxxxxxxxxxxx | |) --- Comment #5 from Peter Gordon <peter@xxxxxxxxxxxxxxxx> 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review