Re: RFS: libapache2-mod-geoip (updated package)
On Fri, May 21, 2010 at 1:50 AM, Paul Wise <pabs@debian.org> wrote:
> On Thu, May 20, 2010 at 5:46 PM, Nigel Jones <dev@nigelj.com> wrote:
>
>> I am looking for a sponsor for the new version 1.2.5-1
>> of my package "libapache2-mod-geoip".
>
> Here is a review:
>
> ftpmaster have overriden the section to httpd, you might want to adopt that:
>
> http://packages.qa.debian.org/liba/libapache2-mod-geoip.html
Done
>
> You might want to switch to debhelper 7 rules.tiny style file.
While I'm using the new tiny rules style in other packages, for now
I'll opt to leave it as-is for now. There are going to be a few
overrides to write I think.
>
> I'm quite surprised apache doesn't have a debhelper script to generate
> the apache module maintainer scripts for you. Please check if there is
> a bug report about that and file one if not.
I'll check into that.
>
> Your source package contains debhelper.log, it should be removed on clean.
Whoops, I've got rid of that, new version looks clean.
>
> The second paragraph of the long description can be merged with the first one.
Done
>
> I would rewrite your changelog entry like this:
Agreed, rewritten it to be a bit clearer.
[snip]
> Once it reaches Ubuntu, you might want to get them to sync so they
> drop their gratuitous changes.
They don't have any changes, just rebuilds.
>
> The upstream INSTALL file says it needs GeoIP >= 1.4.3, you should
> update build-depends in debian/control.
Whoops
>
> Should GeoIPDBFile in the configuration file be uncommented?
I think that is rather debatable there are a few configuration options
for GeoIPDBFile that can be added, by default geoip-database isn't
included by default as end users may have the commercial database or
update it themselves.
I believe this is the reason my libgeoip1 for instance only Recommends
geoip-database.
>
> The Homepage should probably be moved to the source package section of
> debian/control.
Corrected that.
>
> Please review, update and add appropriate debtags:
I've replaced role:shared-lib with role::plugin to better suit.
> lintian complaints:
>
> P: libapache2-mod-geoip source: source-contains-cvs-conflict-copy
> .#mod_geoip.c.1.24
Pedantic mode I'm assuming? Removing this would create a huge patch,
I think this was simply upstream mistake. I don't see much purpose in
removing this as it doesn't effect the end result.
> W: libapache2-mod-geoip: spelling-error-in-changelog intermittant intermittent
Whoops, that was from grabbing the bug title from BTS, I've corrected this.
Should be able to 'dget
http://mentors.debian.net/debian/pool/main/l/libapache2-mod-geoip/libapache2-mod-geoip_1.2.5-2.dsc'
to get the changes.
Thanks,
-- Nigel Jones
Reply to: