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

Re: RFS: endless-sky/0.7.9-1 [ITP]



On Sun, Apr 19, 2015 at 4:33 AM, James Cowgill wrote:
> On Sat, 2015-04-18 at 14:31 -0400, Michael Zahniser wrote:
>>    Dear mentors,
>>
>>    I am looking for a sponsor for my package "endless-sky".
>
> Here are some comments:

I don't intend to sponsor this but here are some further comments:

As you are upstream too, please read through these documents:

https://wiki.debian.org/UpstreamGuide
http://www.freedesktop.org/wiki/Games/Upstream/

Personally I would choose any build system other than scons; I would
recommend autotools or cmake.

The debian/endless-sky.install files can be reduced to this:

debian/endless-sky.install
usr/games
usr/share/applications
usr/share/man
usr/share/icons

debian/control:

I'd suggest running `wrap-and-sort -sa` so that diffs of
debian/control are more readable.

Firefox/Iceweasel say the homepage is insecure as it mixes http
content into a http page. If you change the links to images to https
instead of http that should fix it.

images/asteroid/ images/outfit/ images/planet/station*.png images/ship
endless-sky.iconset/ look like they were automatically rendered from
3D models. It would be a good idea to include the 3D models in the
source code. Bonus points for removing the automatically rendered
images from the VCS and source code zip file and having them rendered
from the 3D model at build time.

images/font/ubuntu*r.png, images/_menu/title.png and images/label look
like they are text rendered from fonts. It would be much better to
render these from TrueType/OpenType fonts at runtime, as this would
enable translators to convert the user-interface to their language. If
you don't want to do that, then these images should be rendered at
build time. If the fonts are in Debian you should depend (or
build-depend) on them, if they are not then they should be packaged
separately.

Personally I would have done the cropping/editing for images/land/*
and images/scene automatically at build time, in case you want to
adjust the cropping/effects in the future or make them higher
resolution.

Some files in images/ui/ say they were produced in Inkscape. If you
didn't throw away the SVG files, please include them in the source.
Bonus points for removing the automatically rendered images from the
VCS and source code zip file and having them rendered from the SVG
files at build time.

Some files in images/land/ say they were produced in the GIMP. If you
didn't throw away the XCF files, please include them in the source.
Bonus points for removing the automatically rendered images from the
VCS and source code zip file and having them rendered from the XCF
files at build time.

You might want to move the *.cpp files to a src/ or source/ directory.

I'd suggest using https in URLs where possible. Everything except the
libjpeg-turbo and sourceforge links and probably the URLs in the plist
files should work with https.

The build process should be verbose (print all the commands and
compiler output) but is not.

The game crashes on my system:

$ endless-sky
#version 120
uniform vec2 scale;
uniform vec2 position;
uniform int glyph;
in vec2 vert;
in vec2 corner;
out vec2 texCoord;
void main() {
  texCoord = vec2((glyph + corner.x) / 96.f, corner.y);
  gl_Position = vec4((vert + position) * scale, 0, 1);
}
0:5(1): error: `in' qualifier in declaration of `vert' only valid for
function parameters in GLSL 1.20
0:6(1): error: `in' qualifier in declaration of `corner' only valid
for function parameters in GLSL 1.20
0:7(1): error: `out' qualifier in declaration of `texCoord' only valid
for function parameters in GLSL 1.20
Shader compilation failed.
terminate called without an active exception
Aborted (core dumped)

Automated checks:

https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git

$ codespell --quiet-level=3
<lots of spelling errors>

$ cme check dpkg
...
Warning in 'control source Build-Depends:2' value 'g++ (>=4.6)':
unnecessary versioned dependency: g++ >= 4.6. Debian has squeeze ->
4:4.4.5-1; wheezy -> 4:4.6.3-8; wheezy -> 4:4.7.2-1; jessie ->
4:4.9.2-2; sid -> 4:4.9.2-2; experimental -> 4:4.9.2-7; experimental
-> 4:5-3;
Warning in 'control source Build-Depends:4' value 'libpng12-dev': This
dependency is deprecated and should be replaced with libpng-dev. See
BTS 650601 for details
Warning in 'control source Build-Depends:5' value
'libjpeg-turbo8-dev': package libjpeg-turbo8-dev is unknown. Check for
typos if not a virtual package.
Warning in 'control source Standards-Version' value '3.9.5': Current
standards version is 3.9.6
...

$ cppcheck -j1 --quiet -f . | grep -vF 'cppcheck: error: could not
find or open any of the paths given.'
[DrawList.cpp:100]: (error) Member variable 'clip' is initialized by itself.
[Point.cpp:250]: (error) Member variable 'v' is initialized by itself.
[Ship.cpp:90]: (error) Possible null pointer dereference: gov

$ fdupes -q -r .
./debian/copyright
./copyright

./images/projectile/skylance+.png
./images/effect/quarg anti missile+0.png

./images/star/g0.png
./images/_menu/g0.png

$ flawfinder -Q -c .
<lots of output, might or might not be useful>

$ find -type f \( -iname '*.c' -o -iname '*.cc' -o -iname '*.cxx' -o
-iname '*.cpp' -o -iname '*.h' -o -iname '*.hh' -o -iname '*.hxx' -o
-iname '*.hpp' \) -exec include-what-you-use {} \;
<lots of output, some might be useful>

$ grep -riE 'fixme|todo|hack|xxx' .
<a bunch of output>

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


Reply to: