Hi Guido,
TL;DR: Most stuff is fine, but for an upload, I'd like to have at least
debian/copright to be streamlined. Fixes for the remaining remarks are
of course appreciated, too. Details see below.
(And if you don't mind that it there will be a slightly different
package in Debian than on SourceForge, I'd prefer that package to have
the version 1.0.6-1, too.)
On Sun, Jun 08, 2014 at 11:21:03AM +0200, Guido van Steen wrote:
> I have just uploaded a new version of Fizsh to Sourceforge. This is version
> 1.0.6. I have also uploaded the corresponding Debian source package to
> Debian Mentors. This package is availabe at
> http://mentors.debian.net/debian/pool/main/f/fizsh
>
> Could you please review the package? I looking forward to reading your
> review.
So far I noticed the following things:
debian/copyright:
* The licenses "BSD-3-clause~zsh-history-substring-search-contributors",
"BSD-3-clause~fizsh" and
"BSD-3-clause~zsh-syntax-highlighting-contributors" seem to be
identical word-wise. In that case the license only needs to be
spelled out respectively defined once.
Of course they differ in the copyright holders, but those are
already listed in the "Files:" starting paragraphs and should only
be listed there.
* The "License:" starting paragraphs only define the license text and
do not need verbatim formatting. They should be word-wrapped if
lines are longer than 80 columns. Comments signs from licenses
copied from source code headers can and should be dropped.
debian/README: This file should be probably better named
debian/README.source.
debian/rules:
* lintian warning script-not-executable
# the scripts in the "./scripts/" directory of the source package are
# copied to both "./debian/usr/bin/" and "./debian/etc/fizsh/". they
# are also copied to "./debian/fizsh/usr/share/fizsh/", but they are
# not meant to be copied there. moreover they end up there without the
# executable bit, which causes lintian to complain about
# "script-not-executable". therefore they are explicitly removed after
# dh_install.
While it is good to not have the files in the package twice, this
maybe a case where it may have been better to override the lintian
warning as the files are meant to be sourced (if it works without
them being executables). This not seldom with shell-related
packages.
So just decided if you want them user-modifiable or not and put them
in either /etc/fizsh or /usr/share/fizsh and override the above
mentioned lintian warning.
Another way to get rid of that lintian warning would be to remove the
shebang lines from those scripts. They're not needed anyway if the
scripts are just sourced.
* A general remark on lines like the following:
[ -d foo ] && rm -rf foo || true
Just using "rm -rf foo" should suffice and is easier to read:
* If the directory doesn't exist, it does nothing
* It does exit with return code 0 even if the file does not exist.
* Warnings from configure:
# without the following override the ./configure script would be called with two
# unrecognized options: --disable-maintainer-mode, --disable-dependency-tracking
override_dh_auto_configure:
./configure \
--prefix=/usr \
--includedir=\${prefix}/include \
--mandir=\${prefix}/share/man \
--infodir=\${prefix}/share/info \
--sysconfdir=/etc \
--localstatedir=/var
I think those warnings can be safely ignored and hence the override
could be dropped. I'm though fine if you are annoyed by the warnings
and prefer to keep the override. (I'd be rather annoyed by such a
long override. :-)
debian/changelog:
* While it is acceptable, I find the style with a blank line between
each item hard to read.
* I'd also indent any line not starting wit "*" by two blanks.
debian/watch and .sig files:
* The .sig files mentioned in the context of pgpsigurlmangle are meant
to be signature files, not public key files as on [1]. See the
documentation for "gpg --sign" for details. The public key file goes
into debian/upstream/signing-key.asc as done correctly with the
package.
[1] http://sourceforge.net/projects/fizsh/files/
I'd be nice if you could fix that at "upstream".
Feel free to ask if my explanations weren't clear enough or too short.
To not only mention negative stuff I noticed, I was also positively
surprised that the package is indeed lintian clean, even with
--pedantic and --experimental!
Yay! :-)
Regards, Axel
--
,''`. | Axel Beckert <abe@debian.org>, http://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE
`- | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
Attachment:
signature.asc
Description: Digital signature