[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#710989: RFS for plover, continued



On Sat, 2013-12-28 at 20:58 +0000, Thomas Thurman wrote:

> So sorry to have vanished.

No worries.

> I've updated the plover packaging to (attempt to) address your concerns
> raised in http://lists.debian.org/debian-mentors/2013/06/msg00026.html .
> Please could you take a further look when you have the time and let me know
> what you think?

Issues blocking upload:

The dependencies of the binary package are missing some things, at
minimum python-wxgtk2.8, python-serial and python-xlib. If upstream were
using setuptools instead of distutils and if debian/rules were using
pybuild, you could have these added automatically.

https://wiki.debian.org/Python/Pybuild

plover tries to use a Python module called appdirs, which doesn't appear
to be packaged for Debian yet.

debian/copyright is incomplete, there are some files not copyright by
Joshua Harlan Lifton and some files not GPL licensed.

osx/plover_template.dmg.gz is a pre-generated file without source that
is not useful on Linux. It would be best to get upstream to remove this
from the tarball and get them to build it in the makefile on MacOS X.

According to the metadata in plover/assets/refresh.png, it was taken
from the tango icon library and was probably generated from an SVG file,
which is missing from the source package. Debian has promised in the
social contract to distribute the source for everything. Sadly
wx.ArtProvider doesn't have a refresh icon or they could just use that.

http://tango.freedesktop.org/Tango_Icon_Library
http://www.debian.org/social_contract

The dictionary appears to be copied from SCOWL, which is already in
Debian. In Debian we prefer that upstreams depend on externally
maintained software (including dictionaries) instead of including copies
of them in their binary packages and tarballs. If upstream wants to
generate a tarball for users of operating systems without proper
repository and dependency systems (aka Windows/MacOS), that is fine, but
it would be nice if they could generate a tarball for us without copies
of any externally maintained code or data.

http://wordlist.sourceforge.net/
http://packages.qa.debian.org/s/scowl.html
https://wiki.debian.org/EmbeddedCodeCopies

Other issues:

Unfortunately none of the keyboards I own support NKRO so I hope that
you do and have tested plover from a user perspective.

plover/oslayer/list_ports_posix.py uses the unsafe function os.popen4
when subprocess isn't available. subprocess was introduced in Python 2.4
so I would suggest upstream switch to requiring it.

doc/plover_guide.pdf was generated from doc/plover_guide.tex by "LaTeX
with hyperref package" and or "pdfTeX-1.40.10" but the build system
doesn't build it from source. The best way to ensure it continues to be
buildable from source in Debian is to build-depend on the right tools
and run them during package build. Preferably upstream would do this
itself and not include the PDF in the source package, since it is not
source, the TeX file is. It also might be useful to install the PDF into
the plover package?

debian/patches/series is empty, you can delete it and the patches dir.

You might want to talk to upstream about releasing OpenPGP signatures
for their releases so that uscan can verify the signatures and you and
Debian can ensure that we have the correct code.

https://wiki.debian.org/debian/watch#pgpsigurlmangle
https://we.riseup.net/riseuplabs+paow/openpgp-best-practices

Once the manual page has been fixed you might want to get it included
upstream.

You might want to use copyright format 1.0 for debian/copyright.

http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/

The Homepage link is a redirect, it might be best to use the right URL.

I noticed the upstream installs in /usr by default. That should be
changed to install to /usr/local by default since /usr is the domain of
the package manager and operating system.

There doesn't appear any build-time testing.

CHANGELOG.rst is empty, upstream should remove it or add to it.

Some files are missing license/copyright information:

http://tieguy.org/blog/2012/03/17/on-the-importance-of-per-file-license-information/

Automatic checks:

https://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package

lintian

P: plover source: debian-watch-may-check-gpg-signature
P: plover: no-upstream-changelog
I: plover: desktop-entry-lacks-keywords-entry usr/share/applications/Plover.desktop

pyflakes

./plover/app.py:20: 'conf' imported but unused
./plover/app.py:22: 'keyboardcontrol' imported but unused
./plover/app.py:26: 'steno_dictionary' imported but unused
./plover/app.py:27: redefinition of unused 'steno' from line 23
./plover/app.py:29: 'load_dictionary' imported but unused
./plover/app.py:31: 'json_dict' imported but unused
./plover/app.py:32: 'rtfcre_dict' imported but unused
./plover/test_translation.py:114: redefinition of unused '_translate_stroke' from line 10
./plover/test_translation.py:114: local variable '_translate_stroke' is assigned to but never used
./plover/steno_dictionary.py:11: 'itertools' imported but unused
./plover/steno_dictionary.py:12: 'normalize_steno' imported but unused
./plover/config.py:9: 'shutil' imported but unused
./plover/main.py:8: 'sys' imported but unused
./plover/main.py:23: local variable 'app' is assigned to but never used
./plover/gui/serial_config.py:14: 'os' imported but unused
./plover/gui/config.py:233: local variable 'dialog' is assigned to but never used
./plover/dictionary/base.py:16: 'CONFIG_DIR' imported but unused
./plover/dictionary/test_rtfcre_dict.py:6: 're' imported but unused
./plover/dictionary/rtfcre_dict.py:100: local variable 'ignore' is assigned to but never used
./plover/dictionary/rtfcre_dict.py:203: local variable 'startpos' is assigned to but never used
./plover/machine/test_stentura.py:129: list comprehension redefines 'b' from line 125
./plover/machine/base.py:11: 'SerialPortException' imported but unused
./plover/machine/base.py:12: 'collections' imported but unused
./plover/machine/stentura.py:661: local variable '_ProtocolViolationException' is assigned to but never used
./plover/machine/test_registry.py:36: undefined name 'machine_registery'
./plover/machine/treal.py:93: local variable 'e' is assigned to but never used
./plover/machine/geminipr.py:52: undefined name 'serial_port'
./plover/oslayer/xkeyboardcontrol.py:159: redefinition of unused 'event' from line 31
./plover/oslayer/xkeyboardcontrol.py:445: local variable 'keycode_events' is assigned to but never used
./plover/oslayer/processlock.py:51: 'tempfile' imported but unused
./plover/oslayer/processlock.py:61: local variable 'user' is assigned to but never used
./plover/oslayer/osxkeyboardcontrol.py:292: local variable 'characters' is assigned to but never used

pep8

lots and lots of warnings

desktop-file-validate

./application/Plover.desktop: error: value "1.1" for key "Version" in group "Desktop Entry" is not a known version

lacheck

"./doc/plover_guide.tex", line 47: Dots should be ellipsis "..."
"./doc/plover_guide.tex", line 97: possible unwanted space at "{"
"./doc/plover_guide.tex", line 128: possible unwanted space at "{"
"./doc/plover_guide.tex", line 130: Whitespace before punctation mark in " ."
"./doc/plover_guide.tex", line 143: possible unwanted space at "{"
"./doc/plover_guide.tex", line 143: possible unwanted space at "{"
"./doc/plover_guide.tex", line 143: possible unwanted space at "{"
"./doc/plover_guide.tex", line 144: possible unwanted space at "{"
"./doc/plover_guide.tex", line 153: possible unwanted space at "{"
"./doc/plover_guide.tex", line 153: possible unwanted space at "{"
"./doc/plover_guide.tex", line 153: possible unwanted space at "{"
"./doc/plover_guide.tex", line 153: possible unwanted space at "{"
"./doc/plover_guide.tex", line 264: <- unmatched "\end{lstlisting}"
"./doc/plover_guide.tex", line 263: -> unmatched "math begin $"
"./doc/plover_guide.tex", line 267: double space at "~ "
"./doc/plover_guide.tex", line 271: Dots should be ellipsis "..."
"./doc/plover_guide.tex", line 292: <- unmatched "\end{document}"
"./doc/plover_guide.tex", line 262: -> unmatched "\begin{lstlisting}"
"./doc/plover_guide.tex", line 293: <- unmatched "end of file ./doc/plover_guide.tex"
"./doc/plover_guide.tex", line 18: -> unmatched "\begin{document}"

isutf8

./doc/SCOWL_README.txt: line 166, char 1, byte offset 49: invalid UTF-8 code
./plover/assets/american_english_words.txt: line 313, char 1, byte offset 7: invalid UTF-8 code

todo

./plover/gui/add_translation.py:        # TODO: add functions on engine for state
./plover/gui/add_translation.py:        # TODO: use state constructor?
./plover/gui/add_translation.py:        # TODO: normalize dict entries to make reverse lookup more reliable with 
./plover/gui/main.py:        # TODO: Figure out why spinner has darker gray background.
./plover/gui/main.py:    # TODO: test all the commands now
./plover/dictionary/base.py:# TODO: maybe move this code into the StenoDictionary itself. The current saver 
./plover/dictionary/base.py:# TODO: write tests for this file
./plover/dictionary/rtfcre_dict.py:# TODO: Convert non-ascii characters to UTF8
./plover/dictionary/rtfcre_dict.py:# TODO: What does ^ mean in Eclipse?
./plover/dictionary/rtfcre_dict.py:# TODO: What does #N mean in Eclipse?
./plover/dictionary/rtfcre_dict.py:# TODO: convert supported commands from Eclipse
./plover/dictionary/rtfcre_dict.py:# TODO: Move dictionary format somewhere more caninical than formatting.
./plover/dictionary/rtfcre_dict.py:# TODO: test this
./plover/dictionary/json_dict.py:# TODO: test this
./plover/app.py:# TODO: unit test this module
./plover/translation.py:    # TODO: combos are not undoable but in some contexts they appear as text. 
./plover/translation.py:    # TODO: Test the behavior of undoing until a translation is undoable.
./plover/logger.py:            # TODO: Figure out what to actually log here.
./plover/test_steno.py:        # TODO: More cases
./plover/steno.py:# TODO: unit test this file
./plover/machine/test_stentura.py:# TODO: add a test on the machine itself with mocks
./plover/machine/base.py:# TODO: add tests for all machines
./plover/machine/base.py:# TODO: add tests for new status callbacks
./plover/machine/base.py:            'port': (None, str), # TODO: make first port default
./plover/machine/passport.py:            'port': (None, str), # TODO: make first port default
./plover/machine/sidewinder.py:# TODO: add options to remap keys
./plover/machine/sidewinder.py:# TODO: look into programmatically pasting into other applications
./plover/machine/sidewinder.py:# TODO: Change name to NKRO Keyboard.
./plover/machine/stentura.py:# TODO: Come up with a mechanism to communicate back to the engine when there
./plover/machine/stentura.py:# TODO: Address any generic exceptions still left.
./plover/machine/stentura.py:TODO: Check that and implement workaround.
./plover/machine/treal.py:# TODO: add tests
./plover/config.py:# TODO: Unit test this class
./plover/oslayer/osxkeyboardcontrol.py:            # Todo(hesky): See if there is a nice way to show the user what's

xxx

./plover/orthography.py:    # defer + ed = deferred (consonant doubling)   XXX monitor(stress not on last syllable)
./plover/machine/passport.py:            # XXX : work around for python 3.1 and python 2.6 differences
./plover/machine/geminipr.py:            # XXX : work around for python 3.1 and python 2.6 differences
./plover/machine/txbolt.py:            # XXX : work around for python 3.1 and python 2.6 differences
./plover/oslayer/list_ports_posix.py:        return 'n/a'    # XXX directly remove these from the list?

-- 
bye,
pabs

http://wiki.debian.org/PaulWise

Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: