On 11/13/2013 12:27 AM, Paul Wise wrote:
On Tue, Nov 12, 2013 at 10:40 PM, Alex Mestiashvili wrote:
I am looking for a sponsor for my package "authprogs"
Since I may want to use this in the future, I've taken a look at it.
A review:
debian/copyright is incorrect, authprogs is GPL-2.0 not GPL-2.0+. If
you want it to be GPL-2.0+ you should talk to upstream about that.
Fixed.
Send upstream a patch to use os.path.expanduser instead of
os.environ['HOME']. The former falls back on the user's home dir from
the password database when HOME is unset.
done, via github
The testsuite uses /tmp instead of tempfile. That should be fixed so
that TMP/TMPDIR are respected and there are no security issues on
multi-user systems. The documentation also mentions /tmp, bad
practices should not be recommended in documentation.
work is still in progress :)
authprogs.html is empty, it should either be fixed or removed.
there is a bug in setup.py, fixed with the patch
Get upstream to fix this duplicated line in authprogs/authprogs.py:
# Authprogs is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# it under the terms of the GNU General Public License v2 as published by
# the Free Software Foundation.
The package FTBFS when built twice in a row, looks like the
authprogs.egg-info dir needs to be removed in debian/rules clean.
Send the patches upstream or change upstream to make them not needed.
Send the lintian authors a patch to update the current Standards-Version.
Check why dh_auto_test doesn't do anything even though there is a test suite.
bump debian/compat to 9
using --buildsystem=pybuild in debian/rules fixes most of the issues above
Check your watch file against the common mistakes (bonus points for
fixing the github examples there):
https://wiki.debian.org/debian/watch#Common_mistakes
fixed, do you mean the not escaped dots in the example:? if yes, than
wiki is updated too.
Get a security review of the upstream code. I checked for obvious
things like os.system()/os.popen()
Once it reaches the archive, upload a screenshot and edit the debtags:
http://screenshots.debian.net/upload
http://debtags.debian.net/edit/authprogs
Automatic checks:
https://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package
lintian:
X: authprogs: duplicate-files usr/share/doc/authprogs/TODO
usr/share/doc/authprogs/TODO.md
dpkg-gencontrol (I think you need to upgrade to pybuild):
warning: package authprogs: unused substitution variable ${python:Versions}
pyflakes:
./authprogs/tests/test_authprogs.py:136: local variable 'dummy_ap' is
assigned to but never used
pep8 complains about lots of these:
E126 continuation line over-indented for hanging indent
E127 continuation line over-indented for visual indent
E128 continuation line under-indented for visual indent
Nice tool, thanks, but it doens't really matter if there is a visually
wrong indentation or ?
I'll try to address the rest of issues soon, but for now I've uploaded a
"fixed" version of authprogs to mentors.
Thank you for the review!
Alex
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org