Re: RFS: mangler (2nd try)
- To: email@example.com
- Cc: "Eugene V. Lyubimkin" <firstname.lastname@example.org>
- Subject: Re: RFS: mangler (2nd try)
- From: Vincent Cheng <email@example.com>
- Date: Sun, 8 May 2011 00:44:56 -0700
- Message-id: <BANLkTikvYWYg+GN=2-JmXdE7FXRnhQFGfirstname.lastname@example.org>
- In-reply-to: <20110507171124.GA19137@r500-debian>
- References: <BANLkTin1BoXmv=KZDKUEnmpF2FtcCjsJqA@mail.gmail.com> <20110507171124.GA19137@r500-debian>
On Sat, May 7, 2011 at 10:11 AM, Eugene V. Lyubimkin <email@example.com> 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
> 5) debian/libventrilo-dev.install: don't install .la-file
> 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 .
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