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
signature.asc
Description: Digital signature
