[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





------- Additional Comments From j.w.r.degoede at hhs.nl  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.