On Sat, May 7, 2011 at 10:11 AM, Eugene V. Lyubimkin <jac...@debian.org> wrote: > [ dropped pkg-voip@ from CC ] > > Here's my review: > > Overall the package is prepared well, thumbs up. Nevertheless: > > 1) The tarball contains directories 'android' and 'iphone'. Repack an > upstream tarball and remove them totally. I don't think anybody would > want check them for redistributability. > Removed. I also implemented a get-orig-source target to do that.
> 2) The source of files src/manglerui.h and icons/mangler-icons.h is unclear > to me. Both look like machine-generated. Are they? If yes, how would one > re-create them? > Yep, both are machine-generated, using makefile snippets from their respective directories. I removed both src/manglerui.h and icons/manglericons.h from the source tarball (and in get-orig-source), and added a override_dh_auto_build target in debian/rules to recreate these files during the build. > 3) Some of libventrilo3/codec-test/* are without copyright notices, which > is suspicious. Are they used in Debian package? If not, better remove > them too. > Removed (and in get-orig-source, of course). > 4) debian/control: please lowercase first letters in short descriptions. > Fixed for the libventrilo packages. However, mangler's description starts off with Ventrilo, which is a name, so I think it should remain capitalized. > 5) debian/libventrilo-dev.install: don't install .la-file > (http://wiki.debian.org/ReleaseGoals/LAFileRemoval). > Fixed. > 6) dpkg-shlibdeps report a number of overlinked libraries, you might want > to pass --as-needed to a linker. > > I'll upload this package once/if at least the points 1-3 and 5 are addressed. > Partly fixed. dpkg-shlibdeps now throws out 4 warnings instead of about a dozen. However, if I remove the packages in build-depends that dpkg-shlibdeps complains about and re-build, I get the following output during the ./configure step: ... checking for speex... yes checking for celt... yes Enabled optional CELT support. checking for xosd_create in -lxosd... no Optional XOSD support is not enabled. checking for new_g15_screen in -lg15daemon_client... no Optional Logitech G15 support is not enabled. checking for espeak_Initialize in -lespeak... no Optional eSpeak support is not enabled. checking for x11... yes checking for xi... yes ... So I reverted my changes. Anyways, at the moment, the only packages I have in my build-depends are debhelper, autotools-dev, and the list of packages given here [1]. So, that's 5/6 issues fixed; hopefully that's enough for an upload. Thanks for taking the time to review my package! :) The package can be found on mentors.debian.net: - URL: http://mentors.debian.net/debian/pool/main/m/mangler - Source repository: deb-src http://mentors.debian.net/debian unstable main contrib non-free - dget http://mentors.debian.net/debian/pool/main/m/mangler/mangler_1.2.2-1.dsc Kind regards, - Vincent [1] http://www.mangler.org/building/ -- 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/BANLkTikvYWYg+GN=2-jmxde7fxrnhqf...@mail.gmail.com