[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: megaglest package



On Tue, Dec 13, 2011 at 4:42 AM, Mark Vejvoda wrote:

> I have worked on transitioning Glest to the new fork called MegaGlest.
> All changes are in svn and would like to know if anyone can take a look
> at it? I had been in contact with pabs but he seems overly busy. I would
> like to get this package accepted while i still have time to work on it.

I did another review based on the latest tarballs you uploaded:

Remaining stuff before I will upload to Debian:

There is a pre-built Windows object file in the megaglest tarball:

source/shared_lib/sources/libircclient/src/libircclient.obj

There are two prebuilt ELF executables in the data source:

egypt/chariot/g2xml in techs/megapack/egypt.7z
egypt/chariot/xml2g in techs/megapack/egypt.7z

The embedded code copy of freetype-gl is still being built and linked
into megaglest despite FTGL being used. This is a waste of build and
user resources.

The megaglest package fails to build if built twice in a row. Looks
like the upstream build system does not delete some generated binaries
on make clean or is missing a make clean. If you don't want to
implement this upstream then you can work-around it in debian/rules.

source/tools/glexemel/g2xml
source/tools/glexemel/xml2g
mk/linux/megaglest_editor
mk/linux/megaglest_configurator
mk/linux/megaglest_g3dviewer
mk/linux/megaglest

New upstream release containing all the above fixes and the ones fixed
in SVN etc.

megaglest/debian/copyright is incorrect, not everything is GPL-3+.
Most things appear to be GPL-2+, some appear to be GPL-1+ and others
name a license that does not exist (but presumably intended to refer
to the GNU GPL). Other files have no copyright/license statement. Some
files are in the public domain, or licenced under the
2-clause/3-clause BSD licenses, LGPL, MIT or other licenses. You can
avoid documenting some of this stuff by removing all the embedded
code/font/flag copies.


Things you might want to fix later:

For the data source pack, just put all the files in the tar.xz instead
of inside .7z files inside the tar.xz.

This is not a proper copyright notice:

"Copyright (c) <year> <copyright holders>"

The manual pages are quite ugly and generate lots of lintian warnings.

pyflakes prints several warnings on the glexemel tool.

Please consider separating all the embedded code copies into a second
tarball, they waste space on every Debian mirror otherwise.

The server code should use mysql prepared statements instead of
escaping. You might want to run some web security scanners over that
too:

http://sectools.org/tag/web-scanners/

Please switch to standard gettext .po files for translations.

Consider switching from the modified embedded code copy of
feathery_ftp to a bittorrent library.

Your embedded code copy of streflop is an old version, you might want
to remove it or update it to 0.3.

You seem to be distributing user-specific settings for a proprietary
IDE, slight waste of space:

source/shared_lib/sources/libircclient/libircclient/libircclient/libircclient.vcproj.SOFTHAUS-XPVM.SoftCoder.user

Please implement automated rendering of the data from the source data.

Please strip the text from the loading screens and render it at
runtime so it can be translated.

Please split the data package into multiple source packages, like the
openarena ones are.

De-duplicate the source data. There are some duplicate blend files,
duplicate textures.

Please de-duplicate the built data. I was able to strip out 100MB by
auto-replacing duplicates with symlinks.

Might want to convert all the source TGA bitmaps to something
losslessly compressed such as PNG, this would save disk space on
user's systems. With optipng/pngcrush/advpng/pngcp on top these could
be much much smaller than the TGA ones.

-- 
bye,
pabs

http://wiki.debian.org/PaulWise


Reply to: