[Bug 238379] Package review: emesene

Fri Dec 21 14:45:50 UTC 2007

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Package review: emesene
Alias: emesene

https://bugzilla.redhat.com/show_bug.cgi?id=238379

------- Additional Comments From me at keimoto.net  2007-12-21 09:45 EST -------
Hi Jason,

It's really like starting over. I've just done some work on this new package
after your valuable feedback. :)

>   emesene.x86_64: W: symlink-should-be-relative /usr/bin/emesene 
>   /usr/share/emesene/emesene
> Why is the executable simply not in /usr/bin to begin with?  If it absolutely
> must reside under /usr/share, the symlink you make must be relative.

I cloned the file instead of symlink now.

>   emesene.x86_64: E: non-executable-script /usr/share/emesene/desktop.py 0644
>   emesene.x86_64: E: non-executable-script /usr/share/emesene/Controller.py 0644
> These scripts start with the usual '#!' lines, but they're not executable.  Are
> they supposed to be?  Many python programmers do this for some unknown reason,
> and we generally allow it, but you should figure out whether the intention is
> for those scripts to ever be executed.

It is intentionally by author which user should run it by the executable 'emesene'.

>   emesene.x86_64: W: summary-not-capitalized emesene is a platform independent 
>   instant messaging client for the Windows Live Messenger (tm) network.
> There's really no reason to put the name of the program in the summeary.  Just
> start with "A platform independent...".

Fixed.

>   emesene.x86_64: E: summary-too-long emesene is a platform independent instant 
>   messaging client for the Windows Live Messenger (tm) network.
> No need for the summary to be a complete sentence.

Fixed.

>   emesene.x86_64: E: description-line-too-long emesene is a platform independent 
>   instant messaging client for the Windows Live Messenger (tm) network.
>   (plus another similar message)
> Keep the lines in the description to around 70 characters.  There's also no need
> to include licensing information in the description; that's what the License:
> tag is for.

Fixed.

>   emesene.x86_64: E: no-binary
>   emesene-debuginfo.x86_64: E: empty-debuginfo-package
> This package is arch-specific, but includes no binaries and contains no debug
> information.  Are you sure it shouldn't be noarch?

Fixed. It should be noarch.

>   emesene.x86_64: W: file-not-in-%lang 
>   /usr/share/emesene/po/ar/LC_MESSAGES/emesen.mo
>   (plus nineteen similar messages)
> It seems that %find_lang was not used to collect the translation files.  Since
> these don't reside in the usual system location (/usr/share/lang) I don't know
> if this is really an issue.  You should investigate.

I was trying to add %find_lang emesene, but rpmbuild complained that the
translation files were not found. It is precisely within
$RPM_BUILD_ROOT/usr/share/emesene/po. (I am temporary commented that out.)

>   emesene.x86_64: W: empty-%post
> Don't include %post if you don't need to put anything there.

Fixed. Added a %{nil}.

> Now, some other issues:
> 
> This package has version "0.1" but the tarball looks like it comes from an SVN
> checkout.  If you're really using a checkout instead of a true upstream release,
> you need to consult both the naming guidelines for proper naming of checkouts:
> http://fedoraproject.org/wiki/Packaging/NamingGuidelines and the guidelines for
> providing instructions for regenerating your tarball:
> http://fedoraproject.org/wiki/Packaging/SourceURL

It is a true upstream release.

> If this isn't a checkout but you're simply downloading a tarball from upstream,
> you need to include the full URL in your Source0: tag.

Fixed. Used a full URL.

> If you're going to use macros like %{__cp}, you need to use them consistently. 
> So either just use "cp" or switch to %{__rm}, %{__ln}, etc.

Fixed. All of them should be macros now.

>
> I'm not sure I understand this from your description: "* Clean and easy to use
> Interface? with no ads".  Why is "Interface" capitalized, and why does a
> question mark follow it?

Fixed. I removed that.

> Your buildroot is not correct; please use one of the recommended values from the
> BuildRoot: section of http://fedoraproject.org/wiki/Packaging/Guidelines.

Fixed. 

> You need to include the COPYING file as %doc.  Currently it's buried with the
code.

Fixed.

> You should have a build dependency on python so that the .py files will get
> byte-compiled properly.
> 
> You should just require "python" instead of "python2"; we don't support any
> release that still has python 1.

Fixed.

> The desktop file is not installed properly; see the "Destop files" section of
> http://fedoraproject.org/wiki/Packaging/Guidelines.  You'll need to include a
> build dependency on desktop-file-utils and then call desktop-file-install as
> shown in the guidelines to properly install the file instead of just copying it.
> You'll also need to fix up any syntax errors; I see that at least your Icon
> directive has an extension and you have an Encoding key, neither of which should
> be there.
> 
> > desktop-file-validate emesene.desktop
> emesene.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated
> emesene.desktop: warning: value "emesene.png" for key "Icon" in group "Desktop
> Entry" is an icon name with an extension, but there should be no extension as
> described in the Icon Theme Specification if the value is not an absolute path

Fixed.

Please kindly review. Thank you very much.

Best Regards,
Caius.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.