Merge lp:~gothicx/entertainer/add_xpm_icon into lp:entertainer

Marco, you need to help us out here. I have no idea why you want to add an xpm icon to Entertainer. You've provided no rationale or synopsis, and for that reason alone, I'm tempted to disapprove this proposal.

You must have a reason for proposing this branch, but my telepathic powers aren't strong enough to read your mind. :)

Please provide some context so I can understand why we should consider adding this file to Entertainer. Is it stated in the Debian Policy Manual that xpm files are needed for all packages that are in the archive?

I've seen plenty of packages that don't include xpm files so you need to sell this concept to me before I consider approving it. References will be helpful to your cause.

In the future, please provide an initial summary comment to your merge proposals or I will be strongly inclined to disapprove the proposal immediately. I don't do it to be mean, but I, as a reviewer, need to understand why a proposed change should be made before I spend any of my free time reviewing some code.

Hi!

First of all, I'm sorry about not commenting it. I'll don't forget in the future. I want the xpm icon, because it's needed/useful for debian / fedora / put_your_distro_here / xfce desktop / gnome, etc. It's only useful for packaging and some desktop managers that doesn't support hicolor icons.

I think lintian warns in debian about not having an xpm icon, so if there are packages without it, it's because Debian Developers ignore it.

Thanks

bom dia Marco,

+1 with Matt for the missing comment.

I'm ok with this new icon if you think that's better from a packaging point of view.

Thanks,
Samuel-

I'm going to continue to play devil's advocate because I still have questions about the purpose of this branch.

1) If xpm files are only for window managers that don't support hicolor, do those window managers support clutter? We use gtk to contain clutter so do these other window managers support gtk. Remember, more importantly, we've only committed to supporting Gnome and Xfce in our purpose statement (e.g., we don't even pretend to be a directly supported KDE application). So unless Xfce doesn't support hicolor icons, then I don't see a need for xpm because we know Gnome supports hicolor.

2) This work seems incomplete because the setup file hasn't changed. I'd guess that just putting the xpm file in the icons directory does not put the file in the correct place for the package. How would running `python setup.py install` put the xpm file in the correct location?

Hi Marco,

Can something happen here?

There's no progress on that discussion so ... we regress :(

Marco, can you provide us a link arguing that this xmp file is needed in Entertainer?

If there's no progress in the next weeks, I'll may remove that proposal to cleanup our proposal queue.

Thanks,
Samuel-

Hi!

I've already talked with Matt about this proposal some time ago at #entertainer channel and we need setup.py to handle this situation. I don't know if he still remember about that. I don't have the channel log about it. He agreed that we need to include it for desktop managers that doesn't support hicolor icons.

Thanks

Great!

So now the question is what do we have to do with this xpm?

Where to copy it?
When to copy it? (every time or only for xfce?)

Matt, Marco do you already have an idea about that?

Samuel-

Hello,

I checked the log and the reason I conceded for including the xpm is not to satisfy desktop managers that don't support hicolor (because there is a good chance that those desktop managers won't support the hardware acceleration needed to run Entertainer). The reason was to satisfy lintian for Debian. But the reason doesn't really matter except to clarify my motivation for landing this branch.

What I was hoping would come out of this branch is that Marco would make the modifications to setup.py needed in order to put the file in the proper place. Since I have not seen any follow up commits, I haven't taken any action to approve the branch. Just adding an xpm file is not going to be good enough. We need to make sure that it gets installed too! :)

In my mind, this branch is simply incomplete in its current state. Hopefully that clears things up for you, Samuel.