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

Re: ITP: lambdacan



Hi Anthony,

On Mo 23 Sep 2013 12:51:05 CEST Anthony Gasperin wrote:

Hi, I did a bit of work on the packaging of lambdacan :
http://anonscm.debian.org/gitweb/?p=debian-edu/lambdacan.git .

Any suggestions, advices ? From my point of view I think the package is
almost ready...

Thanks,
Anthony.


I just looked at the package in lambdacan.git and the overall feedback is: very good work. Still, of course, there is some feedback to give and some work to do. Please note, as this is one of your first official Debian packages, I will be quite picky and verbose on your work. Please see it as support and a learning challenge.

0.
First of all, your package builds fine in a clean Debian unstable chroot. Very good! Make sure you have sbuild or pbuilder set up on your local development machine for testing clean chroot builds of packages (in case this is a coincidence ;-) ).

1.
Once a package is non-option Lintian clean (and lambdacan is), I run Lintian in --pedantic mode. Some issues reported by Lintian then are really really pendantic. Still, the package becomes cleaner when those issues have got fixed.

So ,the command line I use is: ,,lintian -iIE --pedantic --show-overrides --color auto <myarch_changes_file>''. Please do that and get all issues apart from the no-upstream-changelog issue fixed. With the no-upstream-changelog issue I normally do nothing (i.e. keeping it as a reminder that I should contact upstream some time and ask for an upstream changelog file).

2. /debian/changelog
I recommend leaving the upload target / distribution version set to UNRELEASED as long as you do not upload the package (or have it uploaded). See [1] for an example. Check the dch man page for more info on this (search for DEBCHANGE_RELEASE_HEURISTIC).

What I tend to have in my ~/.devscripts file is this

"""
DEBCHANGE_RELEASE_HEURISTIC=changelog
DEBCHANGE_MULTIMAINT_MERGE=yes
DEBCHANGE_MAINTTRAILER=yes
"""

Unnecessary whitespace at EOLs.

Normally, I would expect a blank line between the last changelog entry and the entry footer.

Please use interpunctation in changelog entries:

"""
  * Initial release (Closes: #688901)
"""
->

"""
  * Initial release. (Closes: #688901).
"""

3. /debian/control
The /debian/control file has some whitespaces at the EOLs (esp. in the LONG_DESCRIPTION). Also the LONG_DESCRIPTION could be structured a little bit more (e.g. by adding some paragraphs, maybe).

Compat level 8 and debhelper dependency (>= 7.0.50~) contradict.

SYNOPSIS lines to not end with a dot ('.').

Please add Vcs-*: fields (*= [Git, Browser]) to the Source: block of the control file.

3. /debian/copyright
The upstream author's name in /debian/copyright is not correct. The name is Timothy Jon Fraser. Copyright years are 2005 and 2008. Also you should use a versioned DEP-5 (as reported by pedantic lintian). Please also update the year in your copyright stanza on /debian/*. (We have 2013).

Interestingly, the GPL-3 does not mention the address of the FSF anymore. I wanted to criticize that in your /debian/copyright, but I just checked /usr/share/common-licenses/GPL-3 and the file header example there also lacks the address. So be it then.

4.
With patches (in /debian/patches/) I personally like a numbering scheme for the file name. Just yesterday, I observed a commit from Jonas on zimlib that introduced this scheme:

0xxx_ (e.g. 0001_security-fix.patch, 0002_ftbfs.patch): patches pulled in from upstream.
1xxx_ (e.g. 1001_... etc.): patches interesting for upstream, need reporting
2xxx_ (e.g. 2001_... etc.): Debian specific patches

A short notice on this then should go into /debian/patches/README (see [2]).

Also make sure, that you add patch headers to both patches (already reported by pedantic lintian, see DEP-3 definitions [3]). Minimum header fields needed are Description: and Author:. Also you should mention the patch origin (Origin:) and/or patch forwarding state (Forwarded:).

5. man pages
I like the two man pages very much. Very very good work. Thanks for doing that documentation effort!!!

However, I wonder if the lambdacan.1 man page should better go into man page section 5 (File formats and conventions) or 7 (Miscellanea). See [4] for details and further readings.

6.
Please drop the rules.back file.

7.
No need to have the comment lines in /debian/watch.


That's it for now! Thanks for sticking to this!!!

light+love,
Mike


[1] http://anonscm.debian.org/gitweb/?p=debian-edu/italc.git;a=blob;f=debian/changelog;h=5fe50736b752921751e64ed91c0a7feb03816402;hb=d9e2f4f91f1d26951515fa24c41ad36bb97f920b [2] http://anonscm.debian.org/gitweb/?p=collab-maint/zimlib.git;a=blob;f=debian/patches/README
[3] http://dep.debian.net/deps/dep3/
[4] http://en.wikipedia.org/wiki/Man_page#Manual_sections



--

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
fon: +49 (1520) 1976 148

GnuPG Key ID 0x25771B31
mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb

Attachment: pgp_unzXElZ7f.pgp
Description: Digitale PGP-Unterschrift


Reply to: