[Bug 233783] Review Request: vegastrike-data - Data files for Vega Strike

[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 report.

Summary: Review Request: vegastrike-data - Data files for Vega Strike


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


bugzilla at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|normal                      |medium




------- Additional Comments From peter at thecodergeek.com  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.