On Tue, May 15, 2012 at 12:51:40AM +0200, Arno Töll wrote:
Hi,
>
> this is a review of your package ipset.
>
> * Do not set "DM-Upload-Allowed: yes" on your own. It's your sponsor's
> domain to do so. It's hard enough to find sponsors as is, no need to
> scare off even more potential sponsors by adding DMUA for packages which
> show up on debian-mentors without prior agreement.
Sorry, drop in the latest upload.
> * You declare the debhelper compat[ibility] to be 9, but you build
> depend on "debhelper (>= 9)". Please use a version which actually
> supports the finalized level 9. That is 9.20120115.
I'm following the debhelper(7) manpage which it said that add
debhelper (>= 9) for compat level 9.
However, I adjust it to debhelper (>= 9.20120115~) as your suggestion.
>
> * Do not start with uppercase characters in libipset-dev's description.
> You did it correctly for the other binary package, though.
Adjusted and for libipset2 description also.
> * Why do you use dh_autoreconf? You do not patch automake stuff and a
> brief test seems to confirm it is not needed.
Drop, it's unnecessary as your suggestion.
> * Are you sure about the location of the binary in the file system?
> iptables is in /sbin, why do you install ipset to /usr/sbin?
The ipset depends on libmnl which it is installed in /usr/lib or
/usr/lib/[arch triplet] if it willing support multiarch.
I think, it's not appropriate to install ipset in /sbin when it's still
depends on libmnl that installed in /usr.
> I am willing to sponsor your package if you fix the fist two concerns at
> very least.
Thanks in advance and please review again.
To access further information about this package, please visit the
following URL:
http://mentors.debian.net/package/ipset
Alternatively, one can download the package with dget using this command:
dget -x http://mentors.debian.net/debian/pool/main/i/ipset/ipset_6.12.1-1.dsc
Changes since the last upload:
* Imported Upstream version 6.12.1
* debian/patches/99-ipset-shared-libs.patch:
- Drop as the upstream provides the configuration parameters to force
the ipset binary to link with shared lib.
* debian/patches/90-fix-typo.patch: Add typo fix
* Enable settype dynamic modules support
* debian/rules:
- Add --enable-settype-modules to configure.
All settype modules still build as static which included in libipset
as usual but the ipset binary could runtime load the additional modules
installed at /usr/lib/[arch triplet]/ipset.
* debian/control:
- Add libltdl-dev to build-dep.
* debian/libipset2.symbols: Add symbols file
* debian/control:
- Set debhelper (>= 9.20120115~). (thanks, Arno Töll)
- Drop dh-autoreconf from build-deps as it's unnecessary.
(thanks, Arno Töll)
- Adjust the packages description which should not start with capital
letter. (thanks, Arno Töll)
* debian/rules: Drop autoreconf addon from debhelper configuration.
Best regards,
Neutron Soutmun
Attachment:
signature.asc
Description: Digital signature