Re: RFS: trend

[Paul Wise <pabs@debian.org>]

On Wed, Aug 12, 2009 at 8:37 PM, Yuri D'Elia <wavexx@users.sf.net> wrote:

> I am looking for a sponsor for my package "trend".

A review of the source, binary packages and upstream code:

The configure/configure-stamp targets don't do anything so they can
probably be removed, unless you switch upstream from plain make to
autoconf/automake (which I suggest due to the features it gives you).

Lots of unnessecary whitespace and comments in debian/rules.

debian/rules doesn't handle DEB_BUILD_OPTIONS=noopt or
DEB_BUILD_OPTIONS=parallel=N (see debian-policy).

debian/examples can be reduced to one line: examples/*

The upstream code is LGPLv2+ but the Debian packaging is GPLv3 only,
was that intentional? Generally it is recommended to keep the same
license as upstream for the Debian packaging.

The AUTHORS file doesn't add anything over debian/copyright and
README, I suggest not installing it into the .deb.

IIRC debian policy recommends compiling with -Wall, but trend is not
compiled that way.

When I add -Wall and -Wextra I get these warnings from gcc (not sure
why trend.cc warnings are produced for color.cc):

make[1]: Entering directory `/tmp/buildd/trend-1.0/src'
g++ -MD -Wall -O2 -Wextra  -c -o trend.o trend.cc
g++ -MD -Wall -O2 -Wextra  -c -o color.o color.cc
trend.cc:344: warning: unused parameter 'fd'
trend.cc: In function 'void drawFillZero(const Graph&)':
trend.cc:799: warning: suggest parentheses around comparison in operand of !=
trend.cc: In function 'void drawFillDelta(const Graph&)':
trend.cc:867: warning: suggest parentheses around comparison in operand of !=
trend.cc: At global scope:
trend.cc:1718: warning: unused parameter 'x'
trend.cc:1718: warning: unused parameter 'y'
trend.cc:1829: warning: unused parameter 'state'
trend.cc: In function 'bool parseNums(std::vector<double,
std::allocator<double> >&, char*)':
trend.cc:1920: warning: suggest parentheses around assignment used as
truth value
trend.cc: In function 'bool
parseStrings(std::vector<std::basic_string<char,
std::char_traits<char>, std::allocator<char> >,
std::allocator<std::basic_string<char, std::char_traits<char>,
std::allocator<char> > > >&, char*)':
trend.cc:1930: warning: suggest parentheses around assignment used as
truth value
trend.cc: In function 'void setLimits()':
trend.cc:1419: warning: 'hi' may be used uninitialized in this function
trend.cc: In function 'void drawFillDelta(const Graph&)':
trend.cc:844: warning: 'l2' may be used uninitialized in this function
trend.cc:843: warning: 'l1' may be used uninitialized in this function
trend.cc: In function 'void drawFillZero(const Graph&)':
trend.cc:782: warning: 'last' may be used uninitialized in this function
trend.cc: In function 'size_t drawLine(const Graph&, double)':
trend.cc:714: warning: 'pos' may be used uninitialized in this function
trend.cc: In function 'bool readFNum(FILE*, double&)':
trend.cc:326: warning: 'st' may be used uninitialized in this function
g++   -o trend trend.o color.o -lglut -lGL -lGLU
make[1]: Leaving directory `/tmp/buildd/trend-1.0/src'

Complaints from lintian --info --display-info --display-experimental
--pedantic --show-overrides --checksums --color auto:

P: trend: no-upstream-changelog
I: trend: hyphen-used-as-minus-sign usr/share/man/man1/trend.1.gz:74

You might want to consider using the new debhelper 7 features since
you depend on that version (unfortunately the video for Joey Hess'
DebConf9 talk isn't yet available):

https://penta.debconf.org/dc9_schedule/events/418.en.html

It is fun to do cat /dev/urandom | trend, you might want to include
that in the examples in the manual page.

-- 
bye,
pabs

http://wiki.debian.org/PaulWise