(some of the) Cold Boot Utilities packaged
daniel at debian.org
Wed Aug 27 20:04:53 UTC 2008
[ Disclaimer: a lot of the following comments are sort of 'best
practise' or 'what i consider to be beautiful'. you don't necessarily
have to agree if it doesn't make sense to you ;)
Also, I sometimes don't write the necessary friendly words when
reviewing packages, and i'm not an english native speaker, so, if
anyting sounds harsh below, it isn't meant so. it's just that it lacks
the honey, but i'm totally thankfull and respecting the work you did.
and last but not least, i'm addicted to whitespaces and other nickpicker
stuff, there's no need to worry about my mental sanity here :) ]
Jacob Appelbaum wrote:
the following things i'd like you to change/adjust in your package. note
that those are only formal things, i did not test and lintian-checked
the package yet. if in doubt, or if you want an example, please have a
look at those git repositories of packages, that we have already
uploaded to unstable.
You modified the upstream makefile. this is uncessary, we try to limit
our changes to debian/* only, as good as possible. this makes further
updating and re-using the same debian/* for different versions easier.
so, instead of adding the install call in Makefile, just put it into the
install target of debian rules, or, use a debhelper install file.
your debian rules file should be de-crufted. for newer dpkg versions,
you don't need to define CFLAGS in rules anymore, so you can remove that
policy dictates, that a debian rules file needs to have a couple of
mandatory targets: clean, build, install, binary, binary-arch,
binary-indep. configure* is not mandatory, means, if your package
doesn't make use of it, just remove it completely.
in rules, you have a useless call to dh_link, pleae remove it.
it is considered to be more beautiful by me, that dh_* calls in rules do
not take arguments directly, but that they have their respective
debian/foo file. so, i would replace the 'dh_installexamples samples/'
call with 'dh_installexamples' and add a debian/examples file containing
the line 'samples/'.
remove the useless comments in rules, and re-order the targets
if you don't build anything in your binary-indep target, the binary
target doesn't need to depend on it.
remove the useless empty line at the end of the changelog files.
some people (including me) consider entries in debian changelog to be
complete sentences, ended with a full stop (.).
i personally consider it to beautiful if no file is directly in debian/*
which is *not* interpreted directly by debhelper, so i would move the
manpage to debian/manpage/aesfix.1.
you lack a debian/manpages (which is taken up by dh_installman)
containing the line 'debian/manpage/*' in order to install the manpage
into the package.
this can be done much more elegant if you use -D to your install call,
install -D -m 0755 foo debian/foo/usr/bin/foo
in order to create the necessary directories.
or, if you use an install file, you don't need to worry about creating
the directories yourself too, so just drop that debian/dirs completely.
please bump to 7.
why priority extra? i would use optional here.
maintainer should be set to the team, you should be uploaders.
please bump to debhelper 7.
for policy versions (the number you put into the Standards-Version:
field) is generally only used in a three digit form, not four. please
remove the useless fourth one.
according to the description, you should probably declare a Recommends:
to aeskeyfind and/or rsakeyfind.
please use the proposed machine-interpretable format for debian/copyright.
regarding the license of the debian package, for common-sense reasons, i
strongly advocate to use the same license that upstream uses.
in order to safe some time, it would be nice if you can check the other
packages and adjust the same things there too, and then i look again at
all of them.
Regards and again, thanks a lot for your work,
Address: Daniel Baumann, Burgunderstrasse 3, CH-4562 Biberist
Email: daniel.baumann at panthera-systems.net
More information about the forensics-devel