On Sat, Feb 27, 2016 at 8:16 PM, Philip Rinn wrote:
> Both points are addressed now and updated packaging is uploaded:
> http://mentors.debian.net/package/gtranscribe
I had a play with the app and love the simplicity, very useful for
transcription of single-speaker audio.
I have done a review of the package and am willing to upload if these
two issues are fixed:
The play/pause button doesn't appear to work for me.
I can fairly consistently get it to segfault in Python using this
sequence of events: open a short ogg file, press play, press faster,
crash.
If you want to fix some other things at the same time:
It isn't clear to me what the resume interval is for, does it resume
playing after you press pause for a catch-up break?
I think it would be useful to have speeds less than 0.5 for the slower
typists amongst us.
Starting the app prints this on the terminal:
(gtranscribe:24209): Gtk-CRITICAL **: gtk_widget_grab_default:
assertion 'gtk_widget_get_can_default (widget)' failed
Typing two spaces in a row prints this on the terminal:
** (gtranscribe:27472): CRITICAL **: enchant_dict_check: assertion 'len' failed
With no audio loaded, pressing play/forward/rewind prints this on the terminal:
Traceback (most recent call last):
File "/usr/bin/gtranscribe", line 286, in play_loop
self.set_position_label(duration)
File "/usr/bin/gtranscribe", line 293, in set_position_label
frac = float(self.position) / float(duration)
ZeroDivisionError: float division by zero
With no audio loaded, pressing slower/faster prints this on the terminal:
Traceback (most recent call last):
File "/usr/bin/gtranscribe", line 331, in on_scale_speed_value_changed
fileinfo = FileInfo(self.player.filename, self.md5)
AttributeError: 'gTranscribeWindow' object has no attribute 'md5'
With no audio loaded, pressing pause prints this on the terminal:
Traceback (most recent call last):
File "/usr/bin/gtranscribe", line 263, in play
self._set_update_ui(False)
File "/usr/bin/gtranscribe", line 150, in _set_update_ui
self.play_loop()
File "/usr/bin/gtranscribe", line 286, in play_loop
self.set_position_label(duration)
File "/usr/bin/gtranscribe", line 293, in set_position_label
frac = float(self.position) / float(duration)
ZeroDivisionError: float division by zero
Please add some upstream metadata: https://wiki.debian.org/UpstreamMetadata
You might want to add some AppStream metadata: https://wiki.debian.org/AppStream
It would be great if you could sign your upstream releases with OpenPG
and add the needed things to have uscan automatically verify upstream
tarballs.
https://wiki.debian.org/debian/watch#Cryptographic_signature_verification
Some best practices for OpenPGP you might want to read:
https://help.riseup.net/en/security/message-security/openpgp/best-practices
Your debian/rules seems overly complex, are all the overrides needed?
You might want to use wrap-and-sort to make diffs of the Debian
packaging easier to read. My favourite command-line for this is:
wrap-and-sort --short-indent --wrap-always --sort-binary-packages
--trailing-comma
Automated checks:
build:
WARNING: the following files are not recognized by DistUtilsExtra.auto:
gpl-3.0.txt
lgpl-2.1.txt
lintian:
P: gtranscribe source: debian-watch-may-check-gpg-signature
P: gtranscribe: no-upstream-changelog
check-all-the-things:
$ cme check dpkg
Warning in 'control source Build-Depends:4' value
'python3-distutils-extra (>= 2.22)': unnecessary versioned dependency:
python3-distutils-extra >= 2.22. Debian has wheezy -> 2.36-1;
jessie-kfreebsd -> 2.38-1; jessie -> 2.38-1; stretch -> 2.39-1; sid ->
2.39-1;
$ find -type f \( -iname '*.po' -o -iname '*.pot' \) -exec POFileSpell {} +
<po-file-spell>
<file name="./po/de.po">
...
$ find -type f \( -iname '*.po' -o -iname '*.pot' -o -iname '*.mo' -o
-iname '*.gmo' \) -exec i18nspector {} +
W: ./po/de.po: language-disparity de (pathname) != de_DE (Language header field)
W: ./po/de.po: invalid-date PO-Revision-Date: (empty string)
W: ./po/de.po: no-report-msgid-bugs-to-header-field
$ license-reconcile
...
Copyright mismatch: File gtranscribe/helpers.py: For copyright holder
'Philip Rinn <ri...@inventati.org>' the years 2013-2016 cannot be
fitted into 2013-2014.
Copyright mismatch: File gtranscribe/fileinfo.py: For copyright holder
'Philip Rinn <ri...@inventati.org>' the years 2013-2016 cannot be
fitted into 2013-2014.
...
Copyright mismatch: File gtranscribe/AboutDialog.py: For copyright
holder 'Philip Rinn <ri...@inventati.org>' the years 2013-2016 cannot
be fitted into 2013-2014.
$ find -type f -iname '*.py' -exec pep8 --ignore W191 {} +
./gtranscribe/helpers.py:4:2: W291 trailing whitespace
./gtranscribe/helpers.py:6:68: W291 trailing whitespace
./gtranscribe/helpers.py:8:2: W291 trailing whitespace
./gtranscribe/helpers.py:13:2: W291 trailing whitespace
./gtranscribe/helpers.py:29:80: E501 line too long (101 > 79 characters)
./gtranscribe/helpers.py:34:1: E302 expected 2 blank lines, found 1
./gtranscribe/helpers.py:41:1: W293 blank line contains whitespace
...
$ find -type f -iname '*.py' -exec pyflakes {} +
./gtranscribe/helpers.py:23: 'logging' imported but unused
$ find -type f -iname '*.py' -exec pyflakes3 {} +
./gtranscribe/helpers.py:23: 'logging' imported but unused
$ find -type d \( -iname .bzr -o -iname .git -o -iname .hg -o -iname
.svn -o -iname CVS -o -iname RCS -o -iname SCCS -o -iname _MTN -o
-iname _darcs -o -iname .pc -o -iname .cabal-sandbox -o -iname .cdv -o
-iname .metadata -o -iname CMakeFiles -o -iname _build -o -iname
_sgbak -o -iname autom4te.cache -o -iname blib -o -iname cover_db -o
-iname node_modules -o -iname '~.dep' -o -iname '~.dot' -o -iname
'~.nib' -o -iname '~.plst' \) -prune -o -type f ! \( -iname '*.bak' -o
-iname '*.swp' -o -iname '#.*' -o -iname '#*#' -o -iname 'core.*' -o
-iname '*~' -o -iname '*.gif' -o -iname '*.jpg' -o -iname '*.jpeg' -o
-iname '*.png' -o -iname '*.min.js' -o -iname '*.js.map' -o -iname
'*.js.min' -o -iname '*.min.css' -o -iname '*.css.map' -o -iname
'*.css.min' \) -exec spellintian --picky {} +
./lgpl-2.1.txt: GNU Library Public License -> GNU Library General Public License
./data/ui/gTranscribe.glade: gtk+ -> GTK+
$ grep -riE 'fixme|todo|hack|xxx' .
./bin/gtranscribe: # TODO: Make these configurable
--
bye,
pabs
https://wiki.debian.org/PaulWise
