Hello Paul, Thanks again for the review! I have made further changes, as well in how the upstream file is made. Using the setup.py, this is much simpler! I do have a few comments below:
1.The License lines in debian/copyright are incorrect, please use the right ones from the copyright-format spec. I am a little confused. I checked the link, and it looks like it matches the examples. Is there a specific area that does not match the license format? 2. Could you explain what debian/source/options is for? By having '--extend-diff-ignore' set in this file, I was telling dpkg-source to ignore python generated egg-info. But if this will be done with setup.py sdist, this should not be needed. 3. I added a metadata file, but it might be a little sparse. I was a little confused, as a lot of the data looks to be related to publications? Is any of this needed for this project? I have uploaded the current changes back to Debian Mentors: https://mentors.debian.net/package/tz-converter Please let me know if any further changes are needed. Thanks again for the help! - Dave On 金, 2015-04-24 at 11:11 +0800, Paul Wise wrote: > On Fri, 2015-04-24 at 00:30 +0900, Dave Maiorino wrote: > > > If possible, could I get a further review on these changes? If there is > > anything else that needs to be fixed, please let me know. > > A further review: > > Your upstream tarball contains build artefacts (build/ dir). You should > only ever use `./setup.py sdist` to create upstream tarballs. The > upstream tarball created by that has a debian/ directory in it (that is > missing some files) but it should not have that, please fix setup.py. > > The version in setup.py and PKG-INFO contains both the upstream version > (1.0.0) and the Debian version suffix (the -1) but it should only > contain the upstream version, the Debian version suffix lives only in > debian/changelog. > > data_files in setup.py hard-codes /usr as the prefix. The default for > upstream setup.py should be to use the prefix chosen by the person doing > the install, defaulting to /usr/local. > > icon_path in scripts/tz-converter and tz_converter/main_widget.py > hard-codes /usr as the prefix but it should be using whatever prefix was > passed to setup.py, otherwise no icons will be shown when people install > manually instead of via a Debian package. > > I don't know enough about Python upstream stuff, but this looks weird: > > Platform: UNKNOWN > > The Depends in debian/control contains this python-dateutil, shouldn't > it be python3-dateutil since this is a Python 3 program? > > The Section: python in debian/control is incorrect (this is for Python > modules), please choose one of these, maybe utils or misc? > > https://packages.debian.org/sid/ > > I would suggest upgrading debian/compat to 9 since you have a build-dep > on debhelper 9. > > Please add a debian/upstream/metadata file: > > https://wiki.debian.org/UpstreamMetadata > > The License lines in debian/copyright are incorrect, please use the > right ones from the copyright-format spec: > > https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ > > I'd suggest wrapping debian/watch on the whitespace. > > I don't think the comment in debian/rules is needed any more. > > debian/rules uses --buildsystem=python_distutils but the upstream > setup.py is using setuputils instead. > > Are you sure the override_dh_* in debian/rules are needed? dh should be > able to do all of the things you've overridden automatically. > > The recommended tool for building Python packages these days is pybuild: > > https://wiki.debian.org/Python/Pybuild > > Could you explain what debian/source/options is for? > > Build warnings: > > package init file 'tz_converter/__init__.py' not found (or not a regular file) > dh_pysupport: This program is deprecated, you should use dh_python2 instead. > Migration guide: http://deb.li/dhs2p > > Automatic checks: > > https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git > > $ pep8 --ignore W191 . > <lots of whitespace issues> > > $ pyflakes3 . > ./tz_converter/main_widget.py:3: 'sys' imported but unused > ./tz_converter/main_widget.py:4: 'os' imported but unused > -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
