Code review comment for lp:~rockstar/entertainer/package-structure-apocalypse

Hi Paul,

I'm going to approve this branch as I can see on it the seeds of Entertainer's future.

> Also, most of the backend code is pure crap now, being replaced, and will be
> completely removed. THAT's why I didn't move it to a new location. :)

Okay, with those explanations.

> I don't know yet what to with the system tray icon. No one uses it, it
> doesn't really fit in with what Entertainer is (a media center, not a desktop
> application) and I currently don't have any plans to support it in the local
> server.

Well, I do use it. Also, I think this is not good at all to have *broken* code.
So it turns out to me that either we have to fix this or we remove all the code
related to this notification icon. But, as we're not really sure of the future of
this feature and as fixing this should be rather trivial, I'd prefer the fix
solution.

> They depend on similar things. As I was working on everything else, it only
> made sense to consolidate those dialogs. I think the next logical step would
> be to create an EntertainerDialog, and extend the dialogs that way. That code
> is an utter mess. I think, for the time being, it should stay as the same
> module, and get it cleaned up. If (and this is a big IF) we see a need to
> make
> a package out of this, we can be confident that the code is being maintained.

I'm not sure it's a good idea. I tempted to say "+1 with Matt".
Even if the *one Class per file* rule is nonsense sometime but here ...??

>=== added file 'entertainerlib/download.py'
>--- entertainerlib/download.py 1970-01-01 00:00:00 +0000
>+++ entertainerlib/download.py 2009-05-07 19:37:31 +0000
>@@ -0,0 +1,453 @@
>+'''Downloader classes.'''
>+
>+__licence__ = "GPLv2"
>+__copyright__ = "2009 Entertainer Developers"
>+

I like the idea to group our downloaders. What about weather downloader? Is it
planned in the future?

> self.window.connect('destroy', self.destroy_callback)
>@@ -53,7 +53,7 @@
> except gobject.GError:
> # Must not be installed from a package, get icon from the branch
> file_dir = os.path.dirname(__file__)
>- icon_path = os.path.join(file_dir, '..', '..', '..', 'icons',
>+ icon_path = os.path.join(file_dir, '..', '..', 'icons',
> 'hicolor', '48x48', 'apps', 'entertainer.png')
> icon = gtk.gdk.pixbuf_new_from_file(icon_path)
> self.window.set_icon(icon)

(just me thinking loudly) => If we want to improve our flexibility with folder
tree modifications, we have to find a way to avoid those '..', '..'.
What about adding more xxx_dir (root_dir, data_dir, etc). Those could be calculated
attributes of our config object.

Thanks for the great work Paul. Entertainer will rock with the future client/server
architecture and a robust and powerfull indexer.

Samuel-