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

Re: Bug#670195: RFS: lierolibre/0.1-1



On Tue, Apr 24, 2012 at 5:26 AM, Martin Erik Werner wrote:

> I am looking for a sponsor for my new package "lierolibre"

A review, since you are upstream too, I'm including some advice
related to that too.

I would suggest using git2cl or 'git log' upstream to create the ChangeLog file.

I note many files don't have copyright/license headers:

http://tieguy.org/blog/2012/03/17/on-the-importance-of-per-file-license-information/

I would suggest moving the C++ code to a src/ (or similar) subdir.

I would suggest deleting sdl.m4 from your VCS and just letting
autotools copy in the one from SDL.

How about completely dropping the zlib copy from your gvl fork?

gvl is both an embedded code copy and a nest of embedded code copies,
some of which are even in Debian (zlib, libpcl1-dev, libtut-dev). My
eyes!

I found this post about dtoa.c:

http://patrakov.blogspot.com.au/2009/03/dont-use-old-dtoac.html

Hmm, you are using both the jam and autotools build systems, is that on purpose?

I'd suggest that individual graphics in separate .xpm files would be a
better way to develop the graphics than strips of multiple graphics in
one .xpm file.

You might want to add the old liero release info to NEWS?

On my screen the default window size is so small I can't read the
writing. How about making it 800x600?

I would suggest using one of these algorithms for window scaling:

https://en.wikipedia.org/wiki/Image_scaling
http://research.microsoft.com/en-us/um/people/kopf/pixelart/supplementary/index.html

I would suggest changing the default keyboard settings for player 1 to
the more standard wasd.

The change weapon button doesn't seem to work.

Why the dpkg predepends?

Please use dh --parallel or mention in debian/rules why build
parallelism isn't possible to use.

I would suggest depending on dh-autoreconf and running dh --with
autoreconf to ensure that the configure script and Makefiles are
always buildable on Debian. This is useful since Debian does rebuild
testing for newer versions of autotools.

cppcheck warnings:

[gvl/gvl_test/_build/deque.cpp:51]: (error) Throwing exception in destructor
[gvl/crypt/curve25519.cpp:318]: (error) Memory leak: temp
[gvl/containers/tests/deque.cpp:48]: (error) Throwing exception in destructor
[gvl/crypt/curve25519.cpp:690]: (error) Memory leak: temp1
[gvl/uthread/uthread.cpp:100]: (error) Throwing exception in destructor
[viewport.cpp:132]: (error) Possible null pointer dereference: worm -
otherwise it is redundant to check if worm is null at line 111

-- 
bye,
pabs

http://wiki.debian.org/PaulWise


Reply to: