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 590165 - Should use MC5
Should use MC5
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: General
unspecified
Other Linux
: Normal major
: ---
Assigned To: empathy-maint
empathy-maint
Depends on:
Blocks: 590118 590285 592809
 
 
Reported: 2009-07-29 16:41 UTC by Guillaume Desmottes
Modified: 2009-08-25 12:11 UTC
See Also:
GNOME target: 2.28.x
GNOME version: ---



Description Guillaume Desmottes 2009-07-29 16:41:27 UTC
Empathy should use Mission Control 5.
Comment 1 Guillaume Desmottes 2009-07-29 16:44:03 UTC
These are my commments after a (quick) review of
http://git.collabora.co.uk/?p=user/sjoerd/empathy.git;a=shortlog;h=refs/heads/mc5

• EMPATHY_ARRAY_TYPE_OBJECT: isn't there an equivalent in tp-glib now?
• e700d8f398b27a98ef24476e3909570aa91c329a I don't understand this patch. What's changed?
• e9266689e7b55d9be53afc46c5b3d131ba9837cd would be good to port these tests. At least add a FIXME and open a bug about that
• I think we shouldn't merge MC5 branch while MC4 accounts are not automatically imported when starting the MC5-enabled Empathy for the first time

empathy-account
• empathy_account_update, empathy_account_new, empathy_account_parse_unique_name :  function declaration style is wrong
• empathy_account_got_all_cb: use DEBUG and a more informative message if an error occurs
• empathy_account_requested_presence_cb: same here

empathy-account-manager
• empathy_account_manager_request_global_presence: FIXME is not fixed
• empathy_account_manager_request_global_presence: would be easier/clearer to use empathy_account_is_ready
• emp_account_manager_update_global_presence: a comment explaining the logic of this function would be good
• empathy_account_manager_created_ready_cb   empathy_account_manager_create_account_async  : declaration style is wrong
• empathy_account_manager_created_ready_cb: shouldn't complete the operation with an error if !empathy_account_is_ready (account)? And remove the account?


empathy-dispatcher
• dispatcher_constructor: is Empathy 100% garanteed to be single instance?
• empathy_dispatcher_class_init: useless double \n at the end of the function
• empathy_dispatcher_handle_channels: 2 FIXME

empathy-idle
• empathy_idle_do_set_presence: statusses shouldn't be statuses ?

empathy-account-settings
• empathy_account_settings_set_display_name_async and empathy_account_settings_set_display_name_finish are empty
• style of few declarations is wrong
Comment 2 Xavier Claessens 2009-07-29 17:52:02 UTC

 - avatar cache should be in ~/.cache/Empathy/avatars/<CM>/<protocol>/<account>/<token>
   We could even remove /Empathy/ to share the cache with other clients?
Comment 3 Sjoerd Simons 2009-07-30 17:16:41 UTC
(In reply to comment #1)
> • EMPATHY_ARRAY_TYPE_OBJECT: isn't there an equivalent in tp-glib now?

Nope
> • e700d8f398b27a98ef24476e3909570aa91c329a I don't understand this patch.
> What's changed?

Me failing to include changes in the right commit seems what has happened, i don't really want to rebase at this point though.

> • e9266689e7b55d9be53afc46c5b3d131ba9837cd would be good to port these tests.
> At least add a FIXME and open a bug about that

See 590285

> • I think we shouldn't merge MC5 branch while MC4 accounts are not
> automatically imported when starting the MC5-enabled Empathy for the first >time

No we should merge it asap so people can develop against it, the MC4 importing should be there before the gnome beta release.

> empathy-account-manager
> • empathy_account_manager_request_global_presence: FIXME is not fixed

Fixed in cosimocs branch

> • empathy_account_manager_request_global_presence: would be easier/clearer to
> use empathy_account_is_ready

this code was written before that function existed :)

> • empathy_account_manager_created_ready_cb: shouldn't complete the operation
> with an error if !empathy_account_is_ready (account)? And remove the account?

No, the check is just defensive programming (a notify of the ready property doesn't guarantee that it's true now)

> empathy-dispatcher
> • dispatcher_constructor: is Empathy 100% garanteed to be single instance?
no, it's on my todo list of things to fix

> empathy-account-settings
> • empathy_account_settings_set_display_name_async and
> empathy_account_settings_set_display_name_finish are empty
yes, being implemented in cosimocs branch

The comments i removed from my reply are fixed in the mc5 branch now, thanks for the reivew



Comment 4 Guillaume Desmottes 2009-07-31 09:08:16 UTC
Next release (2.27.90) is on 11th August. If the MC4 importer is merged by then I'm ok to merge the mc5 branch before (a release blocker bug should be opened about that).
Comment 5 Guillaume Desmottes 2009-08-24 13:36:28 UTC
The MC5 port now has its one upstream branch: http://git.gnome.org/cgit/empathy/log/?h=mc5
Comment 6 Guillaume Desmottes 2009-08-25 12:11:27 UTC
Branch has been merged to master \o/