Bug 238379 (emesene) - Package review: emesene
Summary: Package review: emesene
Keywords:
Status: CLOSED NEXTRELEASE
Alias: emesene
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL: http://emesene.org/
Whiteboard:
: emesene-review 444687 (view as bug list)
Depends On: 456671
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-04-30 00:38 UTC by Wilmer Jaramillo M.
Modified: 2018-04-11 14:58 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-08-09 11:07:46 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
srpm 20070116 (595.21 KB, application/x-rpm)
2008-01-15 04:45 UTC, Caius Chance
no flags Details

Description Wilmer Jaramillo M. 2007-04-30 00:38:27 UTC
Spec URL: http://www.linuxtachira.org/review/emesene.spec
SRPM URL: http://www.linuxtachira.org/review/emesene-1.0-1.src.rpm

Description: emesene is an OS independent MSN Messenger client writted in python an GTK. The main idea is to make a client similar to the official MSN Messenger
client but kepping it simple and with a nice GUI.

I need Sponsor

Comment 1 Nigel Jones 2007-04-30 01:09:25 UTC
The spelling needs improving, i.e. the summary should be changed from:
"Emesene is an OS independent MSN Messenger client writted in python and GTK"
to something like:
"Emesene is an OS independent MSN Messenger client written in Python and GTK"

Also note that there is a potential trademark violation with the use of "MSN
Messenger"


Comment 2 Wilmer Jaramillo M. 2007-04-30 01:34:59 UTC
I also was believing the same, neverthless, I consult an similar packages “aMsn”
and "Gaim" in extras: $ rpm -qi amsn gaim

amsn:
"This is an MSN Messenger clone for Unix, Windows, and Macintosh.
It is written in tcl/tk and supports filetransfers, webcam, etc."

gaim:
"Gaim allows you to talk to anyone using a variety of messaging
protocols, including AIM (Oscar and TOC), ICQ, IRC, Yahoo!,
MSN Messenger, Jabber, Gadu-Gadu, Napster, and Zephyr."

these programs also should be there trademark violating?




Comment 3 Wilmer Jaramillo M. 2007-04-30 01:43:41 UTC
(In reply to comment #1)
> The spelling needs improving, i.e. the summary should be changed from:
> "Emesene is an OS independent MSN Messenger client writted in python and GTK"
> to something like:
> "Emesene is an OS independent MSN Messenger client written in Python and GTK"

Done.

Spec URL: http://www.linuxtachira.org/review/emesene.spec
SRPM URL: http://www.linuxtachira.org/review/emesene-1.0-1.src.rpm



Comment 4 Wilmer Jaramillo M. 2007-05-01 07:00:04 UTC
I need sponsor

Comment 5 Matěj Cepl 2007-05-04 21:35:44 UTC
I think there is a difference between "clone of MSN Messenger" and "IM which
allows you talk with anyone using MSN". Just to be on the safe side, I would
stick to the later.

Comment 6 Wilmer Jaramillo M. 2007-05-04 21:55:17 UTC
Probably you are right, I think that “clone” is not determining, since Gaim
provides best _functionality_ at this moment on the protocol that emesene, now,
really emesene is special by you friendly graphical interface, simple and fast.

Comment 7 Mamoru TASAKA 2007-05-24 16:09:19 UTC
Well, first update the URL and your source.

The URL seems to be changed to http://emesene-im.com.ar/forum/index.php
and the version 230507 seems released??

Comment 8 Wilmer Jaramillo M. 2007-05-24 18:16:45 UTC
yes, only that the URL is very speed down, the version 230507 it's being
implemented and tested, and later included into bugzilla link but not included
without testing.

Comment 9 Tyler Owen 2007-06-13 04:55:29 UTC
There is a typo in the Description, says kepping instead of keeping

Other than that:

Rpmlint is quiet
Builds in mock
Installs and runs OK in F7

Comment 10 Wilmer Jaramillo M. 2007-06-13 05:37:49 UTC
Thanks Tyler for you review. 
Updated the spec file: http://www.linuxtachira.org/review/emesene.spec

Comment 11 Wilmer Jaramillo M. 2007-07-11 08:21:57 UTC
Some fixed already for review:
- naming convention for svn checkout present
- Fixed summary, description, and files in %docs 
rpmlint is silent, package builds in mock is ok.

Here is the update:
Spec URL: http://www.linuxtachira.org/review/emesene.spec
SRPM URL: http://www.linuxtachira.org/review/emesene-1.0-2.20070711svn.src.rpm


Comment 12 Jason Tibbitts 2007-07-27 06:30:17 UTC
OK, now that mybashburn is getting close to being done, I'll take a look at
this.  It builds fine and rpmlint finds nothing to complain about.

I can't compare the sources with upstream, because this is a checkout from a
subversion tree.  It's OK to package snapshots like this, but you need to
provide instructions on creating the tarball you are packaging.  See
http://fedoraproject.org/wiki/Packaging/SourceURL

You're using the versioning scheme for post-release snapshots, but according to
the upstream URL it seems to me that version 1.0 has not been released.  Thus
you'd have to use a version-release of something like 1.0-0.2.20070711svn, so
that when 1.0 is finally released you can go to just 1.0-1.  See
http://fedoraproject.org/wiki/Packaging/NamingGuidelines (look for the
"Pre-Release packages" section).

I would remove "It's" from the summary, as I can't think of any summaries that
begin with pronouns.  This isn't a big deal, however.

Your call to desktop-file-install is a little weird; why is it indented like that?

/usr/share/emesene is unowned; change %{_datadir}/%{name}/* in %files to
%{_datadir}/%{name} to fix it.

? Can't compare source with upstream.
X package does not meet naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
? latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* rpmlint is silent.
* final provides and requires are sane:
   emesene = 1.0-2.20070711svn.fc8
  =
   /bin/sh
   dbus-python
   gtk2
   pygtk2
   python
* %check is not present; no test suite upstream.  I installed and ran it; it 
   seemed to start OK but I have no MSN account to actually log in with.
X does not own /usr/share/emesene
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (desktop database update)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

Comment 13 Wilmer Jaramillo M. 2007-08-26 23:38:15 UTC
Hi Jason.

> I can't compare the sources with upstream, because this is a checkout from a
> subversion tree.  It's OK to package snapshots like this, but you need to
> provide instructions on creating the tarball you are packaging.  See
> http://fedoraproject.org/wiki/Packaging/SourceURL

Thank you, i don't know this page, is good information, fixed of course.

> You're using the versioning scheme for post-release snapshots, but according to
> the upstream URL it seems to me that version 1.0 has not been released.  Thus
> you'd have to use a version-release of something like 1.0-0.2.20070711svn, so
> that when 1.0 is finally released you can go to just 1.0-1.  See
> http://fedoraproject.org/wiki/Packaging/NamingGuidelines (look for the
> "Pre-Release packages" section).

Fixed. ;)

> I would remove "It's" from the summary, as I can't think of any summaries that
> begin with pronouns.  This isn't a big deal, however.

Fixed. 

> Your call to desktop-file-install is a little weird; why is it indented like that?

desktop-file-install only permit install emesene.desktop files on
/usr/share/applications/ directory for the menu in desktops environments such as
GNOME or KDE, and update-desktop-database is necessary for update the main menu
with the emesene icon and MIME information in the desktops environments.

> /usr/share/emesene is unowned; change %{_datadir}/%{name}/* in %files to
> %{_datadir}/%{name} to fix it.

Fixed.

> ? latest version is being packaged.

Since svn just the last version tested and stable, soon the _first_ release
candidate of emesene.

Please refresh with the last update:
Spec URL: http://www.linuxtachira.org/review/emesene.spec
SRPM URL: http://www.linuxtachira.org/review/emesene-1.0-0.3.20070711svn.src.rpm

Thanks.

Comment 14 Caius Chance 2007-12-16 06:56:54 UTC
*** Bug 425741 has been marked as a duplicate of this bug. ***

Comment 15 Caius Chance 2007-12-16 06:59:55 UTC
Hi, I have seen this bug hasn't been updated for while. I would like to follow
up the package review (spec modification side) of this package. Rgds, Caius.

Comment 16 Caius Chance 2007-12-16 07:24:32 UTC
http://fedorapeople.org/~cchance/packaging/emesene/

This is the .spec and SRPM based on revision 806, the version after revision
230507. I'll integrate changes according to previous review comments of this bug.

Comment 17 Caius Chance 2007-12-16 10:06:29 UTC
The description of .spec I provided is cloned from emesene official page
http://emesene.org . It has integrated ideas of the comments above:

For comment #1,  there is '(tm)' mark after the "Windows Live Messenger" name.
It should be fine for potential trademark issues.

For comment #5, "emesene is a platform independent instant messaging client for
the Windows Live Messenger (tm) network." <<< It is written in similar way to
the latter.

For comment #7, used the latest URL: http://emesene.org .

For comment #8, used the second latest tarball r806 . The reason is the latest
revision r999 has no executable script of Setup.py (I will update to the latest
tarball after I perform some testing on it.)

For comment #9, testing had been done on my machine:
Rpmlint is quiet
rpmbuild -ba emesene.spec is fine, build is done
Installs and runs OK in "F8"

For comment #11, tested %doc which is okay.

For comment #12, the source tarball used is not a snapshot but a revision (r806)
which is irrelevant to SVN checkout.

=====

Besides the .spec I provided is based on the latest state of emesene, I have
made a gnome start menu entry and patched the executable for convenience to
users. Please kindly review.

Comment 18 Jason Tibbitts 2007-12-16 20:03:35 UTC
I'm perfectly happy for Caius and Wilmer to work on this package if that's you
both want to do.  Just discuss it between yourselves and let me know when you
have a package you'd like for me to look over.

Comment 19 Wilmer Jaramillo M. 2007-12-16 20:21:48 UTC
Hi Jason and Caius, I do not have any problem, Caius should to be the future
packager of emesene if he want, the important thing is to have emesene on Fedora
as soon as possible, I could working now on MyBashBurn too.

Comment 20 Caius Chance 2007-12-16 23:56:02 UTC
Hi Jason, the packages are on http://fedorapeople.org/~cchance/packaging/emesene/

Hi Wilmer, I am alright to be the maintainer of the package, but you have higher
priority to take over that if you want, anytime in the future. I do respect your
great contributions on that, too.

Rgds, Caius.



Comment 22 Jason Tibbitts 2007-12-19 18:14:30 UTC
OK, it looks we're starting over.  The new package needs some work.

First, let's go over the rpmlint output:

  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.

  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.

  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...".

  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.

  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.

  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?

  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.

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

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

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.

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.

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?

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

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

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.

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

Checklist:
X can't check whether source files match upstream.
X package meets naming and versioning guidelines.
X macros are not used consistently in the specfile.
X summary has issues (starts with name of program, ends with dot)
X description has some issues (lines too long, includes license information, 
   etc.) 
* dist tag is present.
X build root not correct.
* license field matches the actual license.
* license is open source-compatible.
X license text is included upstream but not included as %doc in package.
? latest version is being packaged.
X BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
X rpmlint has valid complaints.
X final provides and requires are sane:
   emesene = 0.1-2.fc9
  =
   /bin/sh
   /usr/bin/env
   gnome-python2-extras
   gtk2
?  python2
* %check is not present; no test suite upstream.  I have not yet tested this 
   package.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
X %post scriptlet present but empty.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
X desktop file present but not installed properly.


Comment 23 Caius 'kaio' Chance 2007-12-21 14:45:50 UTC
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.

Comment 25 Caius Chance 2008-01-02 04:19:12 UTC
ping :)

Comment 26 Jason Tibbitts 2008-01-05 01:49:14 UTC
Unfortunately when you set tickets to needinfo they drop way down to the bottom of the bugzilla front page, so this ticket managed to escape my notice when I had free time over the holidays.

Also unfortunately, the package in comment 24 fails to build:

+ desktop-file-install --dir /var/tmp/emesene-1.0-1.fc9-root-mockbuild/usr/share/applications/builddir/build/SOURCES/emesene.desktop
/var/tmp/rpm-tmp.97273: line 36: desktop-file-install: command not found
error: Bad exit status from /var/tmp/rpm-tmp.97273 (%install)

This is because you need a build-time dependency on desktop-file-utils, not a runtime dependency.  So change
  Requires: desktop-file-utils
to
  BuildRequires: desktop-file-utils

Now, that gets things building.  But we're still left with some issues:

  emesene.noarch: E: non-executable-script /usr/share/emesene/desktop.py 0644
  emesene.noarch: E: non-executable-script /usr/share/emesene/Controller.py 0644

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

OK, we'll just ignore those but perhaps you should ask upstream why these are
set up as proper scripts to be executed with #! lines but aren't supposed to
be executable.

  emesene.noarch: W: summary-ended-with-dot A platform independent instant
   messaging client for the Windows Live Messenger (tm) network.
  emesene.noarch: E: summary-too-long A platform independent instant messaging
   client for the Windows Live Messenger (tm) network.
These seem to be new; summary lines aren't sentences and don't end with
periods, and the summary needs to be shorter than 80 characters.  Why not
just:
  Instant messaging client for hte Windows Live Messenger (tm) network
Platform independence doesn't really matter for a Fedora package, since the
package itself obviously isn't.

  emesene.noarch: E: description-line-too-long emesene is a platform
   independent instant messaging client for the Windows Live Messenger (tm)
   network.
Lines in %description need to wrap at 80 characters as well.

  emesene.noarch: W: file-not-in-%lang
   /usr/share/emesene/po/ar/LC_MESSAGES/emesen.mo
  (and several others)
I figured out what the problem is here.  It's complaining that you're not
doing
  %lang(ar) %{_datadir}/emesene/po/ar

and so on for each of the directories files in your %files section.  That
seems like a significant pain, but this package doesn't use the regular
location for locale files and so can't use the standard %find_lang macro.
Unfortunately this does make your %files list quite a bit more complicated
since you have to list everything under %{appdir} separately (or construct a
glob that doesn't include the po directory, but don't forget to include the po
directory itself as %dir %{appdir}/po).

Also, note that the Arabic .mo file seems to be missing an 'e' in its name.  I
don't know if that's a problem.


  emesene.noarch: W: empty-%post
Don't just add %{nil}, which still results in an empty %post.  Remove %post
altogether.  Generally there's no point to empty sections (except for %build,
which has in the past caused some oddities when it's missing).

Finally, I'm not sure I understand why you have
  %verify(not md5 size mtime) %{_bindir}/emesene
in your %files list.  What about that script is expected to change?


Comment 27 Caius Chance 2008-01-07 13:29:22 UTC
Hi Jason, 

Apologies for setting NEEDINFO flag. I didn't expect to have an opposite result.

Fixed all of them, with a little flaw -- files listed trice warning during build.

I'll contact upstream about the Arabic po file incorrect filename and the
permission of file permission issues.

http://fedorapeople.org/~cchance/packaging/emesene/emesene.spec
http://fedorapeople.org/~cchance/packaging/emesene/emesene-1.0-2.fc8.src.rpm

Thank you very much.

Best Regards, Caius.

Comment 28 Caius Chance 2008-01-10 01:01:19 UTC
I've contacted upstream about the:

emesene.noarch: E: non-executable-script /usr/share/emesene/desktop.py 0644
emesene.noarch: E: non-executable-script /usr/share/emesene/Controller.py 0644

Author said that the Controller.py should be 755 and 644 for desktop.py. About
the excessive use of !# line on desktop.py, I've supplied extra info to him for
clarification.

-----

%lang(ar) %{_datadir}/emesene/po/ar

Author told me he'll correct the filename of Arabic l10n files at HEAD. For the
location of such files, I'll look into it after the review approval.

Comment 29 Caius Chance 2008-01-15 01:07:11 UTC
Hi, Jason. :)

Comment 30 Caius Chance 2008-01-15 04:45:41 UTC
Created attachment 291676 [details]
srpm 20070116

Comment 31 Jason Tibbitts 2008-01-15 20:33:37 UTC
FYI, Monday was the 14th, not the 16th.

I'm removing the NEEDSPONSOR blocker because you already have cvsextras access.

Because you have "%{appdir}" in your files list, it recursively includes
everything below it, which means that everything else you explicitly include
under %{appdir} is a duplicate.  You should use
  %dir %{apprdir}
instead, which includes the directory but doesn't automatically pull in
everything beneath it.

This reveals some other issues:
You list %{podir}/sv twice

You don't include any of the files which live directly under %{appdir}

A COPYING file is installed into %{appdir}.  I guess it's not a big problem but
you already included a copy as %doc.

My recommendation for the %files list:

The %doc and %lang stuff you already have (minus the duplicate sv directory)
%dir %{appdir}
%dir %{podir}
%{appdir}/[A-Za-oq-z]*
%{appdir}/plugins_base
%{_bindir}/emesene
%{menudir}/emesene.desktop
%{icondir}/emesene.png

This gets us down to the following three rpmlint complaints:
  emesene.noarch: E: non-executable-script /usr/share/emesene/desktop.py 0644
  emesene.noarch: E: non-executable-script /usr/share/emesene/Controller.py 0644
Honestly I don't really have an issue either way, but according to comment 26
you should chmod 0755 Controller.py.

 emesene.noarch: W: summary-not-capitalized instant messaging client for Windows 
   Live Messenger (tm) network
An easy fix.

Almost done!

Since I'm now reviewing a completely different package from the one I started
out reviewing, I'll run through my checklist again:
* source files match upstream:
   89cbfc144393d5e7aa6766cd429d70891379234c212f477531c1a67ef17dd803  
   emesene-r806.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
? summary is OK (except for capitalization issue)
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
X rpmlint has valid complaints.
* final provides and requires are sane:
   emesene = 0.99-1.fc9
  =
   /bin/sh
   /usr/bin/env
   gnome-python2-extras
   gtk2
   python

* %check is not present; no test suite upstream.  I installed this package and 
   went through the menus and such; everything seemed to work fine, although I 
   have no instant messaging accounts to log into.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
X many duplicates in %files.
X file permissions need a tweak (Controller.py according to upstream).
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* desktop file installed properly and without desktop-file-install complaints.

Comment 32 Caius Chance 2008-01-29 11:36:08 UTC
Hi Jason,

It has been quite a while since last submission. I have fixed the stuffs you
mentioned above:

http://fedorapeople.org/~cchance/packaging/emesene/emesene-0.99-2.fc8.src.rpm
http://fedorapeople.org/~cchance/packaging/emesene/emesene.spec

Thank you very much.

Best Regards, Caius.

Comment 33 Jason Tibbitts 2008-01-30 05:32:41 UTC
Cool.  Still builds OK, and rpmlint says only:
  emesene.noarch: E: non-executable-script /usr/share/emesene/desktop.py 0644
which is OK.

The file permissions look good now, and the duplicates are gone.  The directory
ownership all looks good to me.

APPROVED

Comment 34 Caius Chance 2008-01-30 06:01:06 UTC
New Package CVS Request
=======================
Package Name: emesene
Short Description: Instant messaging client for Windows Live Messenger (tm) network
Owners: cchance
Branches: F-8
InitialCC: cchance
Cvsextras Commits: yes

Comment 35 Kevin Fenzi 2008-01-30 17:12:20 UTC
cvs done.

Comment 36 Caius Chance 2008-01-31 01:16:02 UTC
Built in rawhide.
Built & pushed in F-8.
Thanks everyone.

Comment 37 Caius Chance 2008-05-01 05:43:10 UTC
*** Bug 444687 has been marked as a duplicate of this bug. ***

Comment 38 Caius Chance 2008-05-01 05:44:46 UTC
Hi louiz, please read on how to get sponsorship:

http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Comment 39 Florent Le Coz 2008-07-23 13:28:59 UTC
Spec URL: http://rawen.greystorm.free.fr/Fedora/emesene.spec
SRPM URL: http://rawen.greystorm.free.fr/Fedora/emesene-1.0.1-1.fc9.src.rpm
Description: emesene update (emesene-1.0.1, only bugfixes)
I need a review, and sponsorship
Caius CHANCE wanted to let me 'own' the maintener permission, but for that, I
need sponsorship.
So here is my first package, and I'm trying to do more.
(info: I know well the emesene devs, I'm following emesene developpement almost
everyday, and I wrote some emesene plugins)

Hope I'm doing it right.

Comment 40 Caius Chance 2008-07-28 05:41:18 UTC
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines:

- MUST: The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption on  Package Naming Guidelines . 

<< Please rename emesene_old.spec to emesene.spec.

<< Remove "remove the dummy .pyc .pyo, useless but weighty" line of comment from
spec file and file a bug to include actual commands to perform such action. Or
include that now.

<< Uncomment and change 2 %doc lines to be without "emesene/" prefix to have
documentation packaged properly.

Other than this I didn't see anything wrong at the moment.


Comment 41 Florent Le Coz 2008-07-28 16:41:26 UTC
Spec URL: http://rawen.greystorm.free.fr/Fedora/emesene.spec
SRPM URL: http://rawen.greystorm.free.fr/Fedora/emesene-1.0.1-2.fc9.src.rpm

I made a mistake, the .spec included in the .src.rpm was not the good one.

So, I renamed emesene_old.spec to emesene.spec, I included the doc.
An for the "remove the dummy .pyc .pyo...", this was something I wanted to
investigate, but I discovered that these .pyc and .pyo MUST be in the package,
so, nothing has to be done for this.

Comment 42 Caius Chance 2008-07-28 23:45:50 UTC
Checked files in comment #41 and it looks fine.

Well done, Florent.

Comment 43 Jason Tibbitts 2008-08-09 11:07:46 UTC
I'm not sure why this ticket is open; I've done the review on emesene and things were closed out.  If further things need to happen to this package, a separate ticket should be opened.

Comment 44 Caius Chance 2008-08-10 23:38:18 UTC
As instructed by Fedora Project Wiki, the progress of package maintainer candidate is tracked in package review bug report.

Comment 45 Jason Tibbitts 2008-08-10 23:50:01 UTC
Not this review ticket, though.  This review ticket was originally opened by you when Wilmer submitted emesene.  I reviewed it, it was imported, and the ticket was closed.  It should stay closed; both Wilmer and I are done with the package, and for my part I'd prefer not to see any more review traffic relating to it.  If for some reason the package needs another review, then a new ticket should be opened.  

Packages don't need re-review until after they've been orphaned and dropped from the distro for some period of time, however.


Note You need to log in before you can comment on or make changes to this bug.