Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: vegastrike-data - Data files for Vega Strike https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=233783 bugzilla@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|normal |medium ------- Additional Comments From peter@xxxxxxxxxxxxxxxx 2007-04-22 21:36 EST ------- Here we go; sorry for the lateness of this review. == vegastrike-data 0.4.3-1 == ++ GOOD: * Naming and version/release are good - especially since it's a large data package that can be easily copied in the repository instead of rebuilt per distro. Spec file name is "%{name}.spec" as required. * Game engine/runtime and data packages are separate, with this data-only package being noarch. * License (GPL) is acceptable; and a copy of it is included in the installed documentation (%doc vega-license.txt). * Ownership and permissions of files/directories are sane with no duplicates in the %files listing; and the %defattr line is good. * %changelog section is good. * BuildRoot is properly defined, and cleaned both as the first step in %install and as the only step in %clean. * Dependency list (only the base vegastrike package) is OK. * Successfully builds into binary (noarch) RPMs on both Development and FC-6 (x86_64). * Non-ASCII files are encoded in UTF-8, as required. * Included documentation is good. * Macro and $RPM_* variable usage is consistent. * Package is not relocatable and contains no translations (so %find_lang stuff is not necessary). * Package source matches that of upstream. (The md5sum doesn't match; but a recursive diff between the source checkout as noted in the spec and the unpacked tarball included in your source RPM returns nothing different between the two.) * Files marked as %doc are not required at runtime. ++ BAD: (1) rpmlint complains about several empty files in the source and binary RPMs: I: vegastrike-data checking E: vegastrike-data zero-length /usr/share/vegastrike/units/weapons/weapons.blank E: vegastrike-data zero-length /usr/share/vegastrike/units/factions/factions.template E: vegastrike-data zero-length /usr/share/vegastrike/units/weapons/weapons.template E: vegastrike-data zero-length /usr/share/vegastrike/units/subunits/subunits.blank E: vegastrike-data zero-length /usr/share/vegastrike/units/subunits/subunits.template E: vegastrike-data zero-length /usr/share/vegastrike/units/factions/factions.blank These seem ignorable at first glance though - could you verify this please? (2) As-is, it seems to include its own locally-modified copy of various Python modules (modules/builtin/). A brief perusal of the diff between the included python modules and the system copies of them shows mostly variable renaming and similar generally-insignificant changes. (3) This contains a lot of ISO-8859 text files, as follows. These should be encoded in UTF-8. ./textures/sol2/sources.txt: ISO-8859 text ./accounts/test2.save: ISO-8859 text, with very long lines ./accounts/test1.save: ISO-8859 text, with very long lines ./accounts/default.save: ISO-8859 text, with very long lines ./universe/fgnames/purist.txt: ISO-8859 text ./universe/fgnames/forsaken.txt: ISO-8859 text ./universe/fgnames/LIHW.txt: ISO-8859 text ./universe/fgnames/confed.txt: ISO-8859 text ./universe/fgnames/highborn.txt: ISO-8859 text ./universe/fgnames/shaper.txt: ISO-8859 text ./universe/fgnames/cities.txt: ISO-8859 English text ./universe/fgnames/unadorned.txt: ISO-8859 text ./universe/fgnames/homeland-security.txt: ISO-8859 text ./universe/fgnames/ISO.txt: ISO-8859 text ./universe/fgnames/merchant.txt: ISO-8859 text ./universe/fgnames/andolian.txt: ISO-8859 text (4) The splash_holovid and splash_pacifier animations contain objectionable images (scantily-clad women in rather lude poses). These should probably be removed or replaced with more appropriate content. (5) You make executable every Python file in this which has a shebang. Is this really needed or can the shebang lines be removed instead? (The rest of the scriplets are otherwise sane.) -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review