Bug 205343

Summary: Review Request: cohoba - Cohoba is a GNOME interface for Telepathy.
Product: [Fedora] Fedora Reporter: Sander Hoentjen <sander>
Component: Package ReviewAssignee: Brian Pepple <bdpepple>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhide   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-10-04 21:30:07 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 205031, 207607, 208933    
Bug Blocks: 163779    

Description Sander Hoentjen 2006-09-06 07:33:47 UTC
Spec URL: http://fedora.hoentjen.eu/telepathy/cohoba.spec
SRPM URL: http://fedora.hoentjen.eu/telepathy/cohoba-0.0.3-1.src.rpm
Description: 
Gnome's interfaces for telepathy backend.
The aim of this project is to provide a D-Bus-based framework that unifies 
all forms of real time conversations, including, but not limited to, instant 
messaging, IRC and voice and video over IP. It aims to provide a simple 
interface to client applications allowing them to quickly implement code to 
make use of real time communication over any supported protocol. 

Cohoba is one of the gnome's interfaces for telepathy backend. It aims to be 
innovative and simple to use.

Comment 1 Brian Pepple 2006-09-06 18:58:45 UTC
This should have a desktop file, since it is a gui app.

Comment 2 Sander Hoentjen 2006-09-06 20:48:50 UTC
Ok, I thought it didn't need it because the way you normally start it is because
you add it to the panel. I will fix this soon

Comment 3 Brian Pepple 2006-09-06 21:12:47 UTC
Is it a panel applet?  If so, your right that it doesn't need a desktop file.

I also noticed that it looks like you missing some necessary requires
(gnome-python2 and gnome-python2-applet).

Once python-telepathy gets into FE, I'll do a formal review.

Comment 4 Sander Hoentjen 2006-09-07 05:37:01 UTC
from the README:
-------
Now you can either install it on your system:
sudo make install (or run make install as root)

Then you are able to add the applet through the gnome-panel applet manager.

Or you can try it without installation, by running
./cohoba/cohoba -w
-------
So it is meant to add to the panel, and from there you can open different
windows. If you open it with cohoba -w the applet you normally have will be a
small window that will have the same icon and behave the same as the applet
would, so I could create a desktop file for that. Not sure if I should.

Comment 5 Sander Hoentjen 2006-09-08 12:24:36 UTC
Spec URL: http://fedora.hoentjen.eu/telepathy/cohoba.spec
SRPM URL: http://fedora.hoentjen.eu/telepathy/cohoba-0.0.3-2.src.rpm

added gnome-python2-applet to requires, gnome-python2 will get pulled in as a dep

Comment 6 Brian Pepple 2006-09-08 13:22:01 UTC
I'll do a review on this later today.

Comment 7 Brian Pepple 2006-09-08 21:31:27 UTC
MD5Sums:
9993159e90791fa8ebb57b4b0839e3bb  cohoba-0.0.3.tar.gz

Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Buildroot has all required elements
* All paths begin with macros
* All necessary BuildRequires listed.
* All desired features are enabled
* Package builds in Mock.
* rpmlint produces no error.

Bad:
* There is an ownership problem with the '%{_libdir}/cohoba' directory.
* I believe you still need a requires on pygtk2, since I don't see anything that
will pull in this requirement.
* rpmlint produces the following error:
  E: cohoba only-non-binary-in-usr-lib
* The biggest issue was that I could never create an account to get it working
on FC5.  The problem seemed to be due to account dialog's network spinner button
not being populated.  I was only allowed to add my account & password, and not
the network.

Minor:
* Group Tag should probably be Applications/Communications

Comment 8 Brian Pepple 2006-09-08 21:43:48 UTC
Ok, I found out what the problem was with the account not being able to work. 
You missing a requires on telepathy-gabble.

Comment 9 Sander Hoentjen 2006-09-11 09:02:48 UTC
(In reply to comment #7)
> Bad:
> * There is an ownership problem with the '%{_libdir}/cohoba' directory.
will fix
> * I believe you still need a requires on pygtk2, since I don't see anything that
> will pull in this requirement.
will fix
> * rpmlint produces the following error:
>   E: cohoba only-non-binary-in-usr-lib
Hmm yes this is about /usr/lib64/cohoba/cohoba-applet I guess. Should that
become /usr/libexec/cohoba-applet?
> * The biggest issue was that I could never create an account to get it working
> on FC5.  The problem seemed to be due to account dialog's network spinner button
> not being populated.  I was only allowed to add my account & password, and not
> the network.
see below
> 
> Minor:
> * Group Tag should probably be Applications/Communications
will fix

(In reply to comment #8)
> Ok, I found out what the problem was with the account not being able to work. 
> You missing a requires on telepathy-gabble.
cohoba needs a connection manager, but not necessarily telepathy-gabble. IMHO it
should be fixed upstream (show a message that a connection manager is needed if
none are found).
I know you package gossip, and it will have the same requirement (*a* connection
manager) but for gossip it might make sense to require telepathy-gabble since
that would make upgrading less painful.


Comment 10 Brian Pepple 2006-09-11 13:48:37 UTC
(In reply to comment #9)
> cohoba needs a connection manager, but not necessarily telepathy-gabble. IMHO it
> should be fixed upstream (show a message that a connection manager is needed if
> none are found).
> I know you package gossip, and it will have the same requirement (*a* connection
> manager) but for gossip it might make sense to require telepathy-gabble since
> that would make upgrading less painful.

That's fine, but before this package can be accepted, you should make a patch
then to show a message to the user.  As is, it's almost unusable for the user
who isn't familar with the telepathy framework.

Comment 11 Brian Pepple 2006-09-13 18:23:29 UTC
I was looking at how other distros were packaging cohoba, it looks like most of
them are depending upon telepathy-gabble, which is the sane thing to do for now.
For example: http://packages.ubuntulinux.org/edgy/gnome/cohoba

Comment 12 Sander Hoentjen 2006-09-29 08:27:31 UTC
http://fedora.hoentjen.eu/telepathy/cohoba.spec
http://fedora.hoentjen.eu/telepathy/cohoba-0.0.4-1.src.rpm

I think all issues are fixed, cohoba is updated to latest release which has a
message when no connection managers are found.


Comment 13 Brian Pepple 2006-09-29 12:35:38 UTC
It looks like all the issues in comment #7 & comment #10 have been addressed. 
The only blocker that I see is that the package needs to require the
telepathy-filesystem package since it has files in the
%{_datadir}/telepathy/managers/ directory.  Therefore this bug also depends on
Bug #207607 which has that package.

Comment 14 Sander Hoentjen 2006-10-01 10:38:23 UTC
http://fedora.hoentjen.eu/telepathy/cohoba.spec
http://fedora.hoentjen.eu/telepathy/cohoba-0.0.4-2.src.rpm
this added the require telepathy-filesystem
Be aware that this cohoba version doesn't play nice with current telepathy-gabble.
This is fixed by the not-yet-released new version of telepathy-python. It can
also be fixed by patching telepathy-gabble to include a name= in the manager
file like telepathy-butterfly has. The name= will be depricated in the future if
I understand correctly.

Comment 15 Brian Pepple 2006-10-02 19:23:58 UTC
Ok, it looks like all the outstanding issues in comment #7 and comment #13 have
been fixed.  In addition, since Bug #208933 has been fixed, the problem with
telepathy-gabble in comment #14 is solved.

APPROVED.

Comment 16 Sander Hoentjen 2006-10-04 21:30:07 UTC
thanks for the review