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 ------- Additional Comments From j.w.r.degoede@xxxxxx 2007-04-23 16:41 EST ------- (In reply to comment #1) > Here we go; sorry for the lateness of this review. > > ++ 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? > Yes, I saw those warnings before submission myself too, but I've deliberately ignored them, as I think these empty files might still be needed / usefull. > (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. > Good catch, removed. > (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 > Notice these are data files, not user docs, and I think the game might actually expect / depend on them being ISO-8859, so I've kept these as is. > (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. > These are just 2 of a long list of in game fake advertisements, which are there to create a certain atmosphere. I personally find the ones about guns and joining the army / the ones promoting militarism much more offensive then the 2 you name. IOW this is pretty subjective. Removing any of them feels like applying censorship to me, and lets please not go there unless things are clearly illegal or really bad taste / inappropriate > (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.) Most of these were in the builtin dir, the remaining 2 are really scripts meant to be executed stand alone, and thus should be executable. New version here: Spec URL: http://people.atrpms.net/~hdegoede/vegastrike-data.spec I only updated the specfile as the sources didn't change and it takes eons to upload it with my link. -- 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