Code review comment for lp:~gothicx/entertainer/add_xpm_icon

Revision history for this message
Matt Layman (mblayman) wrote :

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.

review: Needs Fixing

« Back to merge proposal