After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 631946 - decouple roster from Text channel handler
decouple roster from Text channel handler
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Contact List
2.91.x
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
Depends on: 632550
Blocks:
 
 
Reported: 2010-10-12 06:55 UTC by Danielle Madeley
Modified: 2011-08-29 10:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
http://git.Collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/split-text-631946 (18.37 KB, patch)
2010-10-20 09:03 UTC, Guillaume Desmottes
none Details | Review
New version using latest GApplication API (18.64 KB, patch)
2010-10-22 12:35 UTC, Guillaume Desmottes
reviewed Details | Review

Description Danielle Madeley 2010-10-12 06:55:26 UTC
I think we should decouple the roster from the Text channel handler. Especially now with Conn.I.ContactList, which should make accessing our contacts significantly faster.

This is a good next step in optionally replacing the roster with something inside GNOME-Shell.
Comment 1 Guillaume Desmottes 2010-10-12 07:56:30 UTC
Agreed, the text channel handler should probably live in its own process. I just hope that won't decrease performance too much.
Comment 2 Guillaume Desmottes 2010-10-20 09:03:54 UTC
Created attachment 172812 [details] [review]
http://git.Collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/split-text-631946

 data/.gitignore                                    |    2 +-
 data/Empathy.Chat.client                           |   14 ++
 data/Empathy.client                                |   14 --
 data/Makefile.am                                   |    6 +-
 ...esktop.Telepathy.Client.Empathy.Chat.service.in |    3 +
 ...freedesktop.Telepathy.Client.Empathy.service.in |    3 -
 libempathy/empathy-idle.h                          |    1 +
 src/.gitignore                                     |    1 +
 src/Makefile.am                                    |   10 +-
 src/empathy-chat-manager.c                         |  102 +++++++++---
 src/empathy-chat-manager.h                         |    4 +-
 src/empathy-chat-window.c                          |    4 +-
 src/empathy-chat.c                                 |  173 ++++++++++++++++++++
 src/empathy.c                                      |    6 -
 14 files changed, 292 insertions(+), 51 deletions(-)
Comment 3 Guillaume Desmottes 2010-10-22 12:35:52 UTC
Created attachment 172995 [details] [review]
New version using latest GApplication API

 data/.gitignore                                    |    2 +-
 data/Empathy.Chat.client                           |   14 ++
 data/Empathy.client                                |   14 --
 data/Makefile.am                                   |    6 +-
 ...esktop.Telepathy.Client.Empathy.Chat.service.in |    3 +
 ...freedesktop.Telepathy.Client.Empathy.service.in |    3 -
 libempathy/empathy-idle.h                          |    1 +
 src/.gitignore                                     |    1 +
 src/Makefile.am                                    |   10 +-
 src/empathy-chat-manager.c                         |  102 +++++++++---
 src/empathy-chat-manager.h                         |    4 +-
 src/empathy-chat-window.c                          |    4 +-
 src/empathy-chat.c                                 |  174 ++++++++++++++++++++
 src/empathy.c                                      |    6 -
 14 files changed, 293 insertions(+), 51 deletions(-)
Comment 4 Danielle Madeley 2010-10-25 03:10:44 UTC
Review of attachment 172995 [details] [review]:

::: src/Makefile.am
@@ +87,3 @@
 	empathy-av \
+	empathy-auth-client \
+	empathy-chat

$(NULL) on the end?

@@ +123,3 @@
+	empathy-chat-window.c empathy-chat-window.h		\
+	empathy-invite-participant-dialog.c empathy-invite-participant-dialog.h \
+	empathy-chat.c

Here too?

Out of interest. Can any sources be removed from the `empathy` binary?

::: src/empathy-chat.c
@@ +108,3 @@
+  g_option_context_add_main_entries (optcontext, options, GETTEXT_PACKAGE);
+
+  if (!g_option_context_parse (optcontext, &argc, &argv, &error)) {

Brace belongs on next line.

@@ +154,3 @@
+      app_held = FALSE;
+    }
+  else

Should have braces here, since you use them in the 'if' block.
Comment 5 Danielle Madeley 2010-10-25 03:13:00 UTC
Review of attachment 172995 [details] [review]:

::: src/Makefile.am
@@ +87,3 @@
 	empathy-av \
+	empathy-auth-client \
+	empathy-chat

$(NULL) on the end?

@@ +123,3 @@
+	empathy-chat-window.c empathy-chat-window.h		\
+	empathy-invite-participant-dialog.c empathy-invite-participant-dialog.h \
+	empathy-chat.c

Here too?

Out of interest. Can any sources be removed from the `empathy` binary?

::: src/empathy-chat.c
@@ +108,3 @@
+  g_option_context_add_main_entries (optcontext, options, GETTEXT_PACKAGE);
+
+  if (!g_option_context_parse (optcontext, &argc, &argv, &error)) {

Brace belongs on next line.

@@ +154,3 @@
+      app_held = FALSE;
+    }
+  else

Should have braces here, since you use them in the 'if' block.
Comment 6 Guillaume Desmottes 2010-10-25 08:30:44 UTC
Review of attachment 172995 [details] [review]:

::: src/Makefile.am
@@ +87,3 @@
 	empathy-av \
+	empathy-auth-client \
+	empathy-chat

Most of the other SOURCES don't have $(NULL) so I didn't add it (and I'm not convinced it buys us anything tbh).

@@ +123,3 @@
+	empathy-chat-window.c empathy-chat-window.h		\
+	empathy-invite-participant-dialog.c empathy-invite-participant-dialog.h \
+	empathy-chat.c

Nope, I tried but some function are still used.

::: src/empathy-chat.c
@@ +108,3 @@
+  g_option_context_add_main_entries (optcontext, options, GETTEXT_PACKAGE);
+
+  if (!g_option_context_parse (optcontext, &argc, &argv, &error)) {

fixed.

@@ +154,3 @@
+      app_held = FALSE;
+    }
+  else

fixed.
Comment 7 Danielle Madeley 2010-10-25 08:41:23 UTC
(In reply to comment #6)
> 
> ::: src/Makefile.am
> @@ +87,3 @@
>      empathy-av \
> +    empathy-auth-client \
> +    empathy-chat
> 
> Most of the other SOURCES don't have $(NULL) so I didn't add it (and I'm not
> convinced it buys us anything tbh).

Cuts down the size of diffs when you're adding something new. You don't have to remove the last one and replace it with a new-line escape.
Comment 8 Guillaume Desmottes 2010-10-25 09:15:10 UTC
Ok, I've added them.
Comment 9 Guillaume Desmottes 2010-10-25 09:19:36 UTC
Merged to master. Will be in 2.91.2

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.