Bug#706361: gti review

[Dmitry Smirnov <onlyjob@member.fsf.org>]

Dear Felix,

Thanks for packaging this funny software. :)

Isn't the animation is a little too fast? I can hardly recognise the
animated car and it's certainly moves faster than animated train in
`sl`.

I had a look at your package and I'd like to share some suggestions
with you:

There is unnecessary lintian-override for "no-upstream-changelog".
Please remove it. It may be a reminder to generate upstream changelog
if you're producing orig.tar from checkout.

By the way why you didn't forward man page upstream yet?

Package version looks a little bit unconventional. Although it's not a
problem I would suggest to use just version "1.0.4-1" and backport
patches from unreleased upstream commits if necessary. If you don't
want to use upstream tarball releases as is then please provide
get-orig-source target in debian/rules to generate upstream tarball
from checkout.

Perhaps the easiest solution would be to use upstream tarball as is.

## debian/watch

As it is debian/watch could be fine for package version "1.0.4-1".

However it is broken for version "1.0.4+git.20130416.8b0819d-1" which
requires "dversionmangle". Otherwise `uscan` can't download orig.tar.

## debian/rules 

debian/rules contains unnecessary comments (dm-make template
remnants?) -- please consider removing 'em.

Also you might add

    override_dh_builddeb:
        dh_builddeb -- -Zxz

To save some space you might want to compress .deb file with xz
instead of gz. To use xz compression for Debian source you can also
add

   compression = "xz"

to debian/source/options.

## debian/copyright

License name "MIT" is incorrect even though upstream may refer to this
license as such. "MIT" is considered ambiguous by the Free Software
Foundation. copyright-format-1.0 specification recommends to label
newer MIT license as "Expat" however this is an older variant so it
would be better called "MIT-old-style" or similar. See

    http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
    https://en.wikipedia.org/wiki/MIT_License

## debian/control

Priority "extra" is probably better changed to "optional" as per

    http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Priority

IMHO "Suggests: sl" is incorrect because it suggests unrelated package.
I would replace it with "Enhances: git".

Versioned build-depends on "dpkg-dev (>= 1.16.1~)" is unnecessary and
may be completely dropped.


## Patches

Please follow Patch Tagging Guidelines:

       http://dep.debian.net/deps/dep3/

Specifically I think it is a good practice to maintain "Forwarded" and
"Last-Update" headers.

Perhaps it would be

	Forwarded: non-needed

for
	fix-makefile-do-not-strip-symbols.patch
	fix-makefile-installation-path.patch

"fix-makefile-add-dpkg-buildflags.patch" it is done wrong. I
would rename it and modify to respect build flags but without
including file provided my dpkg. This way your improvement will be
usable for upstream and will have to be forwarded.

debhelper 9 in compat 9 mode automatically export build flags which
makes unnecessary to include "/usr/share/dpkg/buildflags.mk" or set
"DPKG_EXPORT_BUILDFLAGS" as we used to do with dh 8 to import
hardening flags.

Best wishes,
 Dmitry Smirnov
 GPG key : 4096R/53968D1B

---

Most people work just hard enough not to get fired and get paid just
enough money not to quit
        -- George Carlin