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

Re: RFS: presage



Hi Matteo,

On Sat, Aug 06, 2011 at 03:12:51PM +0100, Matteo Vescovi wrote:
> Hi Kilian,
> 
> Many thanks for taking the time to review my package.
> 
> >Your changelog looks like this has been in Debian for a looong time yet I
> >cannot find any traces of it. If you reckon this is an initial upload there
> >should be only one entry in debian/changelog.
> 
> Yes, this would be an initial upload to Debian, if accepted. I
> previously uploaded the package for review to debian-mentors, and
> added a changelog entry for each round of reviews.

OMG. ;-)

Anyway, an initial upload is 0.8.6-1 not 0.8.6-3. 
And for a real first uploaded we don't need the entire d-mentors history.
That's on lists.d.o for everyone who wants to have a look.


> >>I would be glad if someone reviewed/uploaded this package for me.
> >Ok, you asked for it, so here goes:
> >
> >1.) debian/control has broken RFC822 multiline fields. The first char of all
> >     subsequent lines needs to be a white space.
> 
> Subsequent lines in multiline fields begin with a single space
> character. I think this complies with RFC822, or am I missing
> something?

My vim highlighting tells me that whitespace is ok but tabstop isn't.
Practicallly dpkg seems to agree with you that tabs are accepted as well.
;-)

For the Description fiels you've used leading white space btw. just not for
Build-Depends and Depends of libpresage1 and pyprompter.

[...]
> >5.) debian/copyright is still at rev 135. Latest candidate of DEP-5 is 174
> >
> >     Moreover there's a typo: License: GLP-2+
> >     And these seem to be not included:
> >     ./apps/gtk/gprompter/scintilla/src/RESearch.h: *No copyright* Public domain
> >     ./apps/gtk/gprompter/scintilla/lexlib/StyleContext.cxx: Public domain
> >     ./apps/gtk/gprompter/scintilla/lexlib/StyleContext.h: Public domain
> 
> Fixed typo and added missing files.

Good.

> BTW, is there a tool I can use to check that debian/copyright is in
> good shape?

None that I know of. Sorry.


> >6.) debian/libpresage1.symbols looks pretty empty
> 
> The symbols exported by the libpresage1 shared library are
> controlled and versioned by a linker version script
> (src/lib/libpresage.map). debian/libpresage1.symbols leverages
> upstream symbol versioning.

Maybe they're controlled, but the libpresage1.symbols has got nothing but
headers! :-?


> >7.) usr/share/doc/libpresage-dev doesn't need to be created in
> >     debian/libpresage-dev.dirs - it'll be auto-created by
> >     debian/libpresage-dev.docs if required.
> 
> Fixed, removed debian/libpresage-dev.dirs

Good.
libpresage-dev.install still has
usr/share/presage/getting_started.txt usr/share/doc/libpresage-dev
which effectively is a *.docs entry. Not sure why you'd want to hide that in
*.install.

[...]
> >10.) pyprompter and libpresage-dev are arch any but none of which has any
> >      arch dependent files. Do you plan to add a static lib or something?
> 
> I changed pyprompter package to arch: all and changed its dependency
> to python-presage (>= ${binary:Version}) to clear the lintian
> not-binnmuable-all-depends-any error. Is this appropriate?

Sure. Looks good to me. Even though as the arch=all only is built for the
first time, you could as well use >= ${source:Version} here.

 
> I'm not sure what to do with the libpresage-dev package.
> I tried changing this to arch: all, but this also caused lintian to
> give a not-binnmuable-all-depends-any error.
> Should -dev packages not be arch: any ?
> Can I change its "libpresage1 (= ${binary:Version})" to "libpresage1
> (>= ${binary:Version})" ?

Mmmmh, this one is a bit more tricky. I think it's best to leave it as is
because otherwise you'd loosen the Depends too much. 

And eventually there'll be a static lib or some other arch dependent code
that you'll want to ship in the -dev package anyway. 

> - dget http://mentors.debian.net/debian/pool/main/p/presage/presage_0.8.6-3.dsc

Your patch
debian/patches/fix-hyphen-used-as-minus-sign-in-text2ngram-man-page.patch
doesn't have a debian/patches/series and therefore isn't applied. Is this
intended?

I'd say it's fine to go in. Do you want to fix the last bits before the
first real upload to Debian or shall we just go as is?

-- 
Best regards,
Kilian

Attachment: signature.asc
Description: Digital signature


Reply to: