Bug#491858: RFS and ITA: jzlib (updated package)
Hi,
Le Wednesday 06 August 2008 23:31:32 Cyril Brulebois, vous avez écrit :
> Cyril Brulebois <kibi@debian.org> (06/08/2008):
> > Since I'd like to familiarize myself with Damien's work, I intend to
> > review this package.
>
> Okay, here we go for a first round:
> - No need to mention the upstream release number in your first
> changelog entry, although it does no harm.
> - You didn't mention bumping debhelper compat level (debian/compat +
> the versioned B-D) from 4 to 5 in your changelog.
> - You didn't mention adding Homepage, Vcs-* either.
> - You should mention you're now shipping examples.
> - Your comment at the top of debian/rules doesn't look like necessary
> to me (although it does no harm).
All done.
> - You could mention you've deleted the override since you fixed the
> copyright file.
> - You could mention you've deleted unneeded files (and which, like
> copyright.in).
> - You could mention you've switched from kaffe.
> - You should mention you're now using ant (and that you've added a
> build.xml file accordingly, at least that's how I understand it).
You're right, debian/changelog is not really clear about those.
Done.
> - Should debian/svn-deblayout be really included in the source package?
> I seem to recall it's possible to set an svn property on the debian
> directory, so that this additional file isn't visible in the source
> package.
Yes, I could use svn-bp:origDir and svn-bp:buildArea but I hardly use svn
properties because I found them difficult to understand for users and others
packagers. IMHO, debian/svn-deblayout is easier to deal with.
> - I tend not to specify “debian uupdate” in my watch files, but I may
> be missing some nice features. Just saying so that you can consider
> whether you need those bits.
AFAIK, with "debian uupdate", uscan will trigger uupdate when a new upstream
version is found / without it just download file and rename it to orig.tar.gz
> - debian/rules again:
> - Not sure the exports are needed (though I didn't build your
> package yet).
I've removed export, not needed.
> - You could use cdbs variables instead of computing package and
> version yourself. Grep for UPSTREAM under /usr/share/cdbs/1/*/* if
> you don't have the docs at hand. Then grep for PACKAGE (probably
> only in the single file you've just found rather than through all
> cdbs files).
You're right. Done.
(CDBS dilema : reading documentation or just grepping files ? :)
Thanks for all your feedback, I really appreciate that.
It will make me take a closer look to my debian/changelog next time ;)
I've uploaded a new package to mentors.debian.net, you should dget it.
Bonne nuit,
--
Damien Raude-Morvan / www.drazzib.com
Reply to: