Bug#804100: RFS: rhythmbox-plugin-alternative-toolbar/0.14.0-1~debian [ITP]

Gianfranco,

many many thanks for spending the time looking at this and giving me some very valuable feedback. It has been much appreciated.

* Package name    : rhythmbox-plugin-alternative-toolbar
   Version         : 0.14.1-1

I've uploaded a newer version with various suggested fixes here:
  http://mentors.debian.net/package/rhythmbox-plugin-alternative-toolbar

  Alternatively, one can download the package with dget using this command:

    dget -x http://mentors.debian.net/debian/pool/main/r/rhythmbox-plugin-alternative-toolbar/rhythmbox-plugin-alternative-toolbar_0.14.1-1.dsc

I've never heard of pyflakes and pyflakes3 - so thanks for introducing me to these tools. I've run these and they no longer throw errors out.

I've run your PEP8 command. The vast majority of the PEP8 issues have now been addressed. For some reason it is picking out whitespace issues with documentation comments.

There are one-or-two slightly too long PEP8 lines left. I've left these since the readability is important.

The find statement is worrying me. All the translations have been exported directly from launchpad.net where the application is actually translated by the wonderful launchpad translation team. I dont really have any control as to the output from launchpad.

Is there a way to "cleanup" these translation po's ? A quick google didnt reveal much.

With regards to the source package - I've introduced a cleanup script on the git project called "debian_cleanup.sh" - this removes the .git folder, .idea folder, the install.sh and the tar.gz file you asked me to remove.

I have run this script before running dh_make --createorig and debuild

The other debian package issues have been addressed (I think).

I've sent an email to the "wnpp" package

- https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=804192

I don't see how I can close this bug report as you requested.

Also - I dont really understand what you want me to add to the changelog file - something like this?

* ITP: 804192

* Initial Debian release

Thanks

David (fossfreedom)

On 5 November 2015 at 17:48, Gianfranco Costamagna <costamagnagianfranco@yahoo.it> wrote:

Control: owner -1 !
Control: tags -1 moreinfo

Hi
let's review:

1) changelog: you need to have only one entry and an ITP bug closed
https://www.debian.org/devel/wnpp/
2) changelog: ~debian makes no sense, please remove
3) compat: 9
4) control: debhelper (>=9)
std-version 3.9.6
priority: optional

5) copyright:
./alttoolbar_rb3compat.py:# Copyright (C) 2012 - Agustin Carrasco

missing

years are outdated "2014" is not good, I would say "xxx-2015" where xxx is the first copyright

Also the first line I guess should use this url
Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
(not sure if they are the same)

6) tarball seems to be not the upstream downloadable from github one
(also it contains the git history)

7) debian/docs: empty?
please add something or drop it
(bonus point, use something to translate README.md into a pdf/html page?)

8) debian/watch file is missing please add one
https://wiki.debian.org/debian/watch

check-all-the-things:

$ grep -riE 'fixme|todo|hack|xxx' .
./alttoolbar_repeat.py: # will be the hacky solution
./alttoolbar_repeat.py: # This is a hacky old method to 'repeat' the current song as soon as it
./alttoolbar_repeat.py: # This is a hacky old method to 'repeat' the current song as soon as it

$ suspicious-source
./img/rb-symbolic-icons.tar.gz

$ pyflakes .
(lot of stuff)

$ pyflakes3 .
(lot of stuff)

$ pep8 --ignore W191 .

(lot of stuff)

$ find -type f $ -iname '*.po' -o -iname '*.pot' $ -exec msgfmt --check --check-compatibility --check-accelerators --output-file=/dev/null {} \;
(lot of stuff)

Please note: some of them might be nitpicks/false positive, please check and report back :)

thanks!

(I know it is a lot of work, but the initial review is always the most difficult for both parts)

cheers,

Gianfranco