Bug 205343
Summary: | Review Request: cohoba - Cohoba is a GNOME interface for Telepathy. | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Sander Hoentjen <sander> |
Component: | Package Review | Assignee: | 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
This should have a desktop file, since it is a gui app. 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 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. 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. 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 I'll do a review on this later today. 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 Ok, I found out what the problem was with the account not being able to work. You missing a requires on telepathy-gabble. (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. (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. 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 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. 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. 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. 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. thanks for the review |