Bug#775509: Seeking Mentor for Bug#775509: RFS: tz-converter/1.0.0 ITP tz-converter
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
>
Reply to: