I don't intend to sponsor this package, but here's my review:
* Łukasz 'sil2100' Zemczak <lukasz.zemc...@canonical.com>, 2013-05-28, 12:04:
http://mentors.debian.net/debian/pool/main/e/evdev/evdev_0.3.2-1.dsc
"evdev" as source package name sounds a bit too generic. Maybe make it
"python-evdev"? Apparently even upstream uses the latter name.
"debhelper (>= 9.0.0)" - debhelper no longer uses such versioning
scheme. Use just "debhelper (>= 9)".
Vcs-* fields are supposed to point to Debian packaging repository, not
to upstream one.
PKG-INFO says "Classifier: Programming Language :: Python :: 3.1" but
debian/control says "X-Python3-Version: >= 3.2". Which one is correct?
Lintian says:
I: evdev source: duplicate-short-description python-evdev python3-evdev
I: evdev source: quilt-patch-missing-description include_license.patch
P: python-evdev: no-upstream-changelog
P: python3-evdev: no-upstream-changelog
(You could easily extract changelog from README.)
"rm -f" already ignores ENOENT, and you don't want to ignore any other
errors. Don't prefix "rm" with "-".
Typo in evdev.device.InputDevice.capabilities docstring:
as a a mapping -> as a mapping
In the get_sw_led_snd() function:
- "max" could be uninitialized;
- the buffer size is too small; it should be (max+7)/8.
- the code doesn't check if malloc(3) returns NULL;
- PyMem_Malloc()/PyMem_Free() should probably be used instead of
malloc(3)/free(3); see: <http://docs.python.org/2/c-api/memory.html>.
But on a second thought, this dynamic allocation is not necessary at
all. LED_MAX, SW_MAX and SND_MAX are all small compile-time constants,
so you could just declare an appropriately-sized array as an automatic
variable.
device_read() returns None if the underlying read(2) call fails. That's
not very pythonic; I'd expect that it throws IOError in such situations.
device_read_many() is similar in this regard.
ioctl_capabilities() and get_sw_led_snd() don't check return value of
ioctl(2).
"except" without exception type is a bad idea:
http://docs.python.org/2/howto/doanddont.html#except
__all__ is supposed to contain only strings.
--
Jakub Wilk
--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20130528220519.ga9...@jwilk.net
