Code review comment for lp://staging/~darkmuggle-deactivatedaccount/ubuntu/oneiric/pep8/oneiric

Revision history for this message
James Page (james-page) wrote :

Hi Ben

I have the following feedback on your merge proposal:

lintian throws some warnings and an error against the source package:

1) debhelper files in debian directory

W: pep8 source: diff-contains-substvars debian/python-pep8.substvars
E: pep8 source: temporary-debhelper-file python-pep8.prerm.debhelper and 1 other

The debian folder contains some temp files created by debhelper; these should be removed:

python-pep8.debhelper.log
python-pep8.postinst.debhelper
python-pep8.prerm.debhelper
python-pep8.substvars

2) Standards-Version is out of date

W: pep8 source: out-of-date-standards-version 3.8.4 (current is 3.9.2)

New upstream releases are a good opportunity to bump the standards version of a package; latest is 3.9.2

See /usr/share/doc/debian-policy/upgrading-checklist.txt.gz for details of what to check but as this is a simple package I would expect minimal/no-change. This should be documented in the changelog as well.

3) dh_python2 transition

Chucks changes from version 0.5.0-1ubuntu1 have been reverted; please re-instate.

See http://wiki.debian.org/Python/TransitionToDHPython2 for more details.

..and a couple against the resulting binary package (see these by running lintian -i -I --show-overrides against the resulting .changes file)

1) Package source format

I: pep8 source: missing-debian-source-format

Ideally the package should specify the source format '3.0 (quilt)' in debian/source/format.

See http://wiki.debian.org/Projects/DebSrc3.0

2) Upstream changelog name

W: pep8: wrong-name-for-upstream-changelog usr/share/doc/pep8/CHANGES.txt

The upstream changelog should be installed to /usr/share/doc/pep8/changelog.gz; you can do this be overriding dh_installchangelogs in debian/rules:

override_dh_installchangelogs:
    dh_installchangelogs CHANGES.txt

You will need to remove it from debian/docs as well. Interestingly enough pkgbinarymangler then removes this file but this is the right way todo so that the distro chooses whether to retain it or not.

Its worth reviewing the lintian output again once these changes are made as it may reveal other warnings/errors.

You can just re-push your changes back to this branch and the merge proposal will update.

Cheers

James

review: Needs Fixing

« Back to merge proposal