Hi,

Le Wednesday 06 August 2008 23:31:32 Cyril Brulebois, vous avez écrit :
> Cyril Brulebois <[EMAIL PROTECTED]> (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




--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to