Bug 241792 - (empathy) Review Request: Empathy - An instant messaging client built using Telepathy
Review Request: Empathy - An instant messaging client built using Telepathy
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Brian Pepple
Fedora Package Reviews List
:
Depends On: 241530 241791
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-30 13:30 EDT by David Nielsen
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-06-18 20:31:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
bdpepple: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
Mock build log (11.57 KB, text/plain)
2007-06-04 10:22 EDT, Brian Pepple
no flags Details

  None (edit)
Description David Nielsen 2007-05-30 13:30:50 EDT
Spec URL: http://lovesunix.net/fedora/empathy.spec
SRPM URL: http://lovesunix.net/fedora/empathy-0.5-1.fc8.src.rpm
Description: Empathy consists of a rich set of reusable instant messaging widgets, and a GNOME client using those widgets. It uses Telepathy and Nokia's Mission Control, and reuses Gossip's UI. The main goal is to permit desktop integration by providing libempathy and libempathy-gtk libraries. libempathy-gtk is a set of powerful widgets that can be embeded into any GNOME application.
Comment 1 Peter Gordon 2007-05-30 20:58:05 EDT
Hi, David.

I'd be happy to review this for you. However, someone with proper sponsorship
permissions will need to sponsor you into the Account System groups when it is
done (if you're not already sponsored).

Before I push this through a mock build though, I see some things in the spec
file that seem wrong:

(1) Your desktop-file-install invocation puts the .desktop file into the
autostart directory; meaning that it will be started at every desktop session.
It'd be better, I believe, if you install it to the global
%{_datadir}/applications directory. In that way, the user can choose to add it
to their session startup as normal if they wish. (

(2) Your %files listing makes the package own everything in
%{_datadir}/empathy/, but not the directory itself. If you change that to the
directory (remove the asterisk), then rpm-build will automagically know to own
that directory and everything it contains, recursively.

Thanks.
Comment 2 Matěj Cepl 2007-05-30 21:24:16 EDT
(In reply to comment #1)
> If you change that to the directory (remove the asterisk), then rpm-build will
> automagically know to own that directory and everything it contains,
> recursively.

That should be "remove the slash", right?
Comment 3 Brian Pepple 2007-05-30 21:57:17 EDT
(In reply to comment #1)
> I'd be happy to review this for you. However, someone with proper sponsorship
> permissions will need to sponsor you into the Account System groups when it is
> done (if you're not already sponsored).

Peter, I'm willing to do the final review for this, and be David's sponsor.  I
probably won't be able to get to it until tomorrow evening though. :)

BTW, telepathy-mission control needs to be reviewed and approved first.

Comment 4 Peter Gordon 2007-05-30 23:18:13 EDT
(In reply to comment #2)
> (In reply to comment #1)
> > If you change that to the directory (remove the asterisk), then rpm-build will
> > automagically know to own that directory and everything it contains,
> > recursively.
> 
> That should be "remove the slash", right?

Not necessarily. The trailing slash when listed in the %files is only aesthetic
AFAIK. rpm-build will still know it's a directory through the magic of ... file
magic! :)

(In reply to comment #3)
> > I'd be happy to review this for you. However, someone with proper sponsorship
> > permissions will need to sponsor you into the Account System groups when it is
> > done (if you're not already sponsored).

> Peter, I'm willing to do the final review for this, and be David's sponsor.  I
probably won't be able to get to it until tomorrow evening though. :)

> BTW, telepathy-mission control needs to be reviewed and approved first.

I like that plan. :D

I'll do the review for mission-control, then; and when I'm done I can do a
preliminary review of this and leave the final YEA/NAY up to you.

Thanks, Brian.
Comment 5 David Nielsen 2007-05-31 02:17:59 EDT
Putting the .desktop file in autostart is upstream behavior, in an effort to
stick as close to upstream as possible that is what I would like to continue
doing. There is a decent argument to be made for starting your IM client with
your session instead of by demand, it's an always on kind of thing and it seems
a better plan to me to work on addressing cases where we do not wish to be
present online such a meetings by way of integration with presence frameworks
like Galago.
Comment 6 Sindre Pedersen Bjørdal 2007-05-31 08:44:50 EDT
Seeing as I'm submitted the mission control and was about to submit an Empathy
package before David beat me to the punch, I'd like to do both Empathy and
mission-control as a co-maintainership with David. 
Comment 7 David Nielsen 2007-05-31 11:24:36 EDT
I was talking to Matej about doing a small Telepathy team so we could get better
coverage of the Telepathy packages in Fedora, but that sound like a good idea to
you?
Comment 8 Sindre Pedersen Bjørdal 2007-05-31 16:06:58 EDT
Sounds like a good idea. I'm in.
Comment 9 David Nielsen 2007-06-01 12:19:26 EDT
Spec URL: http://lovesunix.net/fedora/empathy.spec
SRPM URL: http://lovesunix.net/fedora/empathy-0.5-2.fc8.src.rpm

Fixed the ownership and shut up rpmlint, I'll stick to upstream with regards to
the .desktop files unless it's strongly preferred that I don't do so - the
biggest problem I see is that it's a bit difficult to restart empathy when it
crashes.. naturally the solution here is to make empathy stop crashing.
Comment 10 David Nielsen 2007-06-01 17:51:49 EDT
Spec URL: http://lovesunix.net/fedora/empathy.spec
SRPM URL: http://lovesunix.net/fedora/empathy-0.6-1.fc8.src.rpm

Bump to version 0.6 - no changes aside that.
Comment 11 Peter Gordon 2007-06-01 23:48:53 EDT
Brian: As suggested, I'm reassigning this to you for review.

David/Sindre: I like the idea as well; perhaps we could form a specific Special
Interest Group ("SIG") for it?
Comment 12 Brian Pepple 2007-06-04 10:22:34 EDT
Created attachment 156078 [details]
Mock build log

Hmm, fails to build in Mock.  If I get some free time today, I'll try to figure
out why.
Comment 13 Mamoru TASAKA 2007-06-04 10:42:48 EDT
(In reply to comment #12)
> Created an attachment (id=156078) [edit]
> Mock build log
> 
> Hmm, fails to build in Mock.  If I get some free time today, I'll try to figure
> out why.

This type of build failure is usually due to missing BR for
gettext, I suppose.
Comment 14 Brian Pepple 2007-06-04 10:47:41 EDT
(In reply to comment #13)
> (In reply to comment #12)
> > 
> > Hmm, fails to build in Mock.  If I get some free time today, I'll try to figure
> > out why.
> 
> This type of build failure is usually due to missing BR for
> gettext, I suppose.

That's what I assume also, but I haven't had time to verify it.
Comment 15 Sindre Pedersen Bjørdal 2007-06-04 12:07:17 EDT
Created a Telepathy SIG page on the wiki:
http://fedoraproject.org/wiki/SIGs/Telepathy
Comment 16 David Nielsen 2007-06-04 14:12:49 EDT
Spec URL: http://lovesunix.net/fedora/empathy.spec
SRPM URL: http://lovesunix.net/fedora/empathy-0.6-2.fc8.src.rpm

Added BuildRequires for gettext and it now builds in mock here... provided I
grokked mock right.

Comment 17 Brian Pepple 2007-06-04 16:14:31 EDT
MD5Sums:
d8b51a02718f4e28a9916d54f3a9a3c0  empathy-0.6.tar.bz2

Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Buildroot has all required elements
* All paths begin with macros
* All directories are owned by this or other packages (except item in Bad section)
* All necessary BuildRequires listed.
* All desired features are enabled
* Make succeeds even when %{_smp_mflags} is defined
* Builds fine in mock.
* rpmlint produces only the following warning, which can be safely ignored:
 W: empathy non-conffile-in-etc /etc/gconf/schemas/empathy.schemas

Bad:
 * missing Requires on telepathy-filesystem.  this makes sure
%{_datadir}/telepathy/managers is correctly owned.

Strongly Urge Changing:
 * There is no guideline that I'm aware of that states you can't use the
autostart directory.  IMHO though this is a REALLY poor idea to do on this package.

Ok, now that I've got that out of the way.

+1 APPROVE, provided that you add the telepathy-filesytem as a requires before
importing into cvs.  Also, as I mentioned earlier I'm willing to be your sponsor.

http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b
Comment 18 David Nielsen 2007-06-04 20:05:23 EDT
Thank you and consider the minor issues pointed out fixed.

New Package CVS Request
=======================
Package Name: empathy
Short Description: GNOME Instant Messaging Client based on Telepathy
Owners: david@lovesunix.net
Branches: FC-6 F-7
InitialCC: 
Comment 19 Brian Pepple 2007-06-04 20:45:10 EDT
David, since I'm your sponsor could you add me to the acl list for empathy after
you import it into CVS.  Thanks.
Comment 20 Tom "spot" Callaway 2007-06-05 11:15:21 EDT
cvs done.
Comment 21 Brian Pepple 2007-06-18 20:31:14 EDT
David, has this been built yet?  If so, you can close this bug as NEXTRELEASE.
Comment 22 David Nielsen 2007-06-18 22:56:41 EDT
Upsie.. forgot about this one, Empathy has been in all the repos for a bit now,
baring a bit of trouble with F-7 and the insane updates-testing mess.
Comment 23 David Nielsen 2007-07-10 12:03:33 EDT
I'm orphaning this one, the documentation does not say to whom I have to assign
this but here is the CVS change request.

Package Change Request
======================
Package Name: empathy
[Updated Fedora Owners: ]
Comment 24 Kevin Fenzi 2007-07-10 19:18:18 EDT
cvs done.
Comment 25 Peter Gordon 2007-08-13 04:42:23 EDT
I'd like to unorphan and take ownership of Empathy, now that I've got Telepathy
working nicely with AIM. Thanks.

Package Change Request
======================
Package Name: empathy
Updated Fedora Owners: peter@thecodergeek.com

Comment 26 Peter Gordon 2007-08-13 16:03:17 EDT
Oops; forget to set the fedora-cvs flag...
Comment 27 Kevin Fenzi 2007-08-13 18:56:18 EDT
cvs done.
Comment 28 Peter Gordon 2007-10-09 23:03:23 EDT
Package Change Request
======================
Package Name: empathy
New Branches: F-8

I'd like to start building Empathy with its experimental VoIP support  enabled
for people to test/break/debug/etc. in order to have it as a potential feature
for F9+. I'd like a branch for F-8 that can be kept "stable" please. :) Thanks.


Comment 29 Kevin Fenzi 2007-10-10 18:23:26 EDT
cvs done.

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