Re: RFS: mangler (2nd try)
On Sat, May 7, 2011 at 10:11 AM, Eugene V. Lyubimkin <jackyf@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/
Reply to: