Hi Vincent, Thanks for the review! Questions & comments below:
On Sun, Aug 18, 2013 at 9:27 AM, Vincent Bernat <ber...@debian.org> wrote: > Hi Tom! > > Glad to see you are working on this package. > > debian/control: why do you depend explicitely on such a version of GCC? > The README states gcc 4.7+ but your version excludes GCC as in Wheezy. > I chose to use the version that I built with (though in retrospect I don't think I should've had that tilde in there), thinking that if it's in testing now it should be readily available on unstable, but I didn't think about wheezy. I've updated the Build-Depends to look like this: Build-Depends: debhelper (>= 8.0.0), gcc (>= 4.7.0~) ... > debian/copyright: just to let you know that it is usually easier to use > the same license for debian/* than for the package. For example, this > would allow upstream to pick your eventual patches without having having > a license conflict. But this is not a necessity. > Ah, of course -- I've changed it to 2-clause BSD. > capnproto-doc.docs, capnproto-doc.install, capnproto.install have some > odd contents. For .install, it would be the first time I see a directory > without a wildcard in it. Maybe this works but this seems unusual to me. > Hm. I don't create a separate -doc package -- is this necessary for landing this package? If not, I'd be inclined to remove the *-doc.* files all together. FWIW, the only docs available in the upstream package is a brief README (at least in the latest release tarball) & the only docs available for this package atm is the manpage I wrote for the capnp command. I actually got the directory-without-a-wildcard thing from the protobuf package: tom@desktop:~/Source/protobuf-2.4.1$ cat debian/protobuf-compiler.install usr/bin Seems to work fine, but I can change it if it's at odds with the usual style. > README.source content is bogus too, remove it. > Done. > In debian/rules, remove the comments saying this is a sample. This is no > longer a sample. > Also done. > In C++, the symbols file will change depending on the > architecture. Therefore, you should use demangled names by using > c++filt. See: https://wiki.debian.org/UsingSymbolsFiles > I followed the instructions on the wiki page, but there still seems to be some mangled names in the symbols file, e.g. the first few lines here: (c++)"_ZN2kj10StringTree6concatIJNS_8ArrayPtrIKcEES0_NS_10FixedArrayIcLm1EEEEEES0_DpOT_@Base" 0.2.0 (c++)"_ZN2kj10StringTree6concatIJNS_8ArrayPtrIKcEES4_S0_EEES0_DpOT_@Base" 0.2.0 (c++)"kj::StringTree::StringTree(kj::Array<kj::StringTree>&&, kj::StringPtr)@Base" 0.2.0 (c++)"kj::StringTree::StringTree(kj::Array<kj::StringTree>&&, kj::StringPtr)@Base" 0.2.0 Is that a problem? I've got to head out for a while, but I'll have a read through the rest of that documentation later today & see if there's anything enlightening in there. > I am unable to compile the package on a clean chroot. The unittests > fail: > > <snip> > Weird -- I'll try that out myself & see if I can figure out what's going wrong. Cheers, Tom -- Tom Lee / http://tomlee.co / @tglee -- To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/CAKwFPQ-EoLBdF-F3PeUgRf=j4p6e+7j-neotcm1pd8xuppn...@mail.gmail.com
