> Hello Michael Budde! It seems like we were working on the same bug #379266 in
> parallel, so a lot of our work is similar, if not identical (earcandy.desktop
> has the exact same comment!).
> So I'm going to comment on the changes you've made versus what's now in the
> trunk (lp:earcandy revision 50).
> 

Yeah, I noticed your branch the day after I started my branch. I've certainly 
got some inspiration from your branch, hope you don't mind that ;)

> I find your approach to the runnable script earcandy interesting. You
> basically moved the code under if __name__ == "__main__" from the EarCandy.py
> to the script. Theoretically, this is the best method, practically, I've found
> from my personal experience with Epidermis that it makes namespaces rather
> confusing. The other approach which I took calls the script by running
> subprocess.call(['python', ear_candy.py']), creating two python processes one
> depending on the other, which is a bit of a waste but makes the result in
> System Monitor prettier. The final approach (that I can think of) is to launch
> it by running subprocess.Popen(['earcandy.py']) which is lighter on resources
> but less pretty in System Monitor.

I'm not sure what you mean when you say that it "makes namespaces rather confusing".
Do you have any examples where your method would be prefered to mine?

> Since you're using distutils, the data files will be separated from the .py
> files, so you created a function called find_data_file, similar to my
> find_program_file. You check to see in which directory of /usr/ or /usr/local/
> the files are installed. This actually superfluous, they're always going to be
> under sys.prefix, according to the distutils documentation.

Running setup.py in the source tree gives me this:
$ ./setup.py -n install
running install
[..snip..]
running install_data
creating /usr/local/share/earcandy
copying data/earsLabel.png -> /usr/local/share/earcandy
copying data/pulseoptions.glade -> /usr/local/share/earcandy
copying data/settings.xml -> /usr/local/share/earcandy
copying data/earsLabelOff.png -> /usr/local/share/earcandy
copying earcandy.desktop -> /usr/local/share/applications
creating /usr/local/share/pixmaps
copying data/earsLabel.png -> /usr/local/share/pixmaps
running install_egg_info
Writing /usr/local/lib/python2.6/dist-packages/earcandy-0.5.egg-info

Also I don't quite like how you've copied find_program_file() to every source file
that needs it. Right now it's a pain to edit that function as you have to do it in three
places. Why not put it in a separate file and import it?

> Having a seperate EarCandyInfo.py file for the application's metadata is
> certainly interesting. Having one place to modify the program's version number
> is nice, but do you really need it for things like APPNAME?

You're right. That probably is overkill.

> I'm going to go ahead and comment on your Ubuntu packaging branch too,
> lp:~mbudde/earcandy/ubuntu revision 59 if you don't mind. You included a
> dependency which I somehow forgot: pulseaudio, well done! Having the Ubuntu
> packaging in a separate branch from the trunk is an interesting idea, and one
> which a lot of projects go by. I personally prefer having the debian/ folder
> in the original source, keeps the developers responsible for the packaging and
> things like dependencies.

I'm just going to quote https://wiki.ubuntu.com/BzrMaintainerHowto because I have
no personal opinion on this :)
    "If the project is not Ubuntu specific and other distributions want to use it
    too, you should register the branch as trunk, and create another branch ubuntu
    which adds the packaging bits on top of it. Code development happens on trunk,
    which is just merged into ubuntu whenever appropriate (such as when
    a new upload is prepared)."

> Obviously, I'm not the developer of EarCandy, it'll be up to KillerKiwi to
> decide what to take and what to keep. Thanks for contributing!

Thank you very much for taking your to make this review. It's always nice with
some input!