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 677465 - Use TpProtocol more
Use TpProtocol more
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Accounts
2.33.x
Other Linux
: Normal enhancement
: ---
Assigned To: empathy-maint
empathy-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-05 12:21 UTC by Guillaume Desmottes
Modified: 2012-06-06 09:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
empathy_account_settings_get_tp_protocol: return a TpProtocol (2.50 KB, patch)
2012-06-06 08:48 UTC, Guillaume Desmottes
committed Details | Review
empathy_account_settings_check_readyness: use the existing TpProtocol to list params (2.84 KB, patch)
2012-06-06 08:48 UTC, Guillaume Desmottes
committed Details | Review
accounts_widget_generic_setup: continue iterating when treating optional param in simple mode (927 bytes, patch)
2012-06-06 08:48 UTC, Guillaume Desmottes
committed Details | Review
account-settings: use TpProtocol's API to get TpConnectionManagerParam (4.64 KB, patch)
2012-06-06 08:48 UTC, Guillaume Desmottes
committed Details | Review
use tp_list_connection_managers_async() (2.98 KB, patch)
2012-06-06 08:48 UTC, Guillaume Desmottes
committed Details | Review
protocol-chooser: use TpProtocol (3.73 KB, patch)
2012-06-06 08:48 UTC, Guillaume Desmottes
committed Details | Review
import-pidgin: use TpProtocol (1.45 KB, patch)
2012-06-06 08:48 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2012-06-05 12:21:10 UTC
A bunch of tp-glib API have been deprecated. We should stop using them and rely more on TpProtocol.
Comment 1 Guillaume Desmottes 2012-06-06 08:48:25 UTC
Created attachment 215715 [details] [review]
empathy_account_settings_get_tp_protocol: return a TpProtocol
Comment 2 Guillaume Desmottes 2012-06-06 08:48:28 UTC
Created attachment 215716 [details] [review]
empathy_account_settings_check_readyness: use the existing TpProtocol to list params

No need to request another one.
Comment 3 Guillaume Desmottes 2012-06-06 08:48:30 UTC
Created attachment 215717 [details] [review]
accounts_widget_generic_setup: continue iterating when treating optional param in simple mode

We may have more non-optional parameters later.
Comment 4 Guillaume Desmottes 2012-06-06 08:48:33 UTC
Created attachment 215718 [details] [review]
account-settings: use TpProtocol's API to get TpConnectionManagerParam
Comment 5 Guillaume Desmottes 2012-06-06 08:48:36 UTC
Created attachment 215719 [details] [review]
use tp_list_connection_managers_async()
Comment 6 Guillaume Desmottes 2012-06-06 08:48:38 UTC
Created attachment 215720 [details] [review]
protocol-chooser: use TpProtocol
Comment 7 Guillaume Desmottes 2012-06-06 08:48:41 UTC
Created attachment 215721 [details] [review]
import-pidgin: use TpProtocol
Comment 8 Xavier Claessens 2012-06-06 09:15:17 UTC
(In reply to comment #2)
> Created an attachment (id=215716) [details] [review]
> empathy_account_settings_check_readyness: use the existing TpProtocol to list
> params
> 
> No need to request another one.

Something looks wrong in indentation in the patch:
-  if (priv->protocol_obj == NULL)
+    if (priv->protocol_obj == NULL)

wondering if that's mixing tabs and spaces
Comment 9 Xavier Claessens 2012-06-06 09:19:14 UTC
Review of attachment 215719 [details] [review]:

::: libempathy/empathy-connection-managers.c
@@ +254,3 @@
+
+  g_signal_emit (self, signals[UPDATED], 0);
+  g_object_unref (self);

you should add a tp_weak_ref_destroy() here, no?
Comment 10 Xavier Claessens 2012-06-06 09:20:58 UTC
The rest looks good
Comment 11 Guillaume Desmottes 2012-06-06 09:33:39 UTC
(In reply to comment #8)
> (In reply to comment #2)
> > Created an attachment (id=215716) [details] [review] [details] [review]
> > empathy_account_settings_check_readyness: use the existing TpProtocol to list
> > params
> > 
> > No need to request another one.
> 
> Something looks wrong in indentation in the patch:
> -  if (priv->protocol_obj == NULL)
> +    if (priv->protocol_obj == NULL)
> 
> wondering if that's mixing tabs and spaces

I just miss re-idented when I moved the code around, fixed.


(In reply to comment #9)
> Review of attachment 215719 [details] [review]:
> 
> ::: libempathy/empathy-connection-managers.c
> @@ +254,3 @@
> +
> +  g_signal_emit (self, signals[UPDATED], 0);
> +  g_object_unref (self);
> 
> you should add a tp_weak_ref_destroy() here, no?

You're right; good catch. Fixed.
Comment 12 Guillaume Desmottes 2012-06-06 09:36:14 UTC
Attachment 215715 [details] pushed as ef7ff08 - empathy_account_settings_get_tp_protocol: return a TpProtocol
Attachment 215716 [details] pushed as daf7a5b - empathy_account_settings_check_readyness: use the existing TpProtocol to list params
Attachment 215717 [details] pushed as c92e84c - accounts_widget_generic_setup: continue iterating when treating optional param in simple mode
Attachment 215718 [details] pushed as 505305a - account-settings: use TpProtocol's API to get TpConnectionManagerParam
Attachment 215719 [details] pushed as 0e095b8 - use tp_list_connection_managers_async()
Attachment 215720 [details] pushed as 899be99 - protocol-chooser: use TpProtocol
Attachment 215721 [details] pushed as 3ac6462 - import-pidgin: use TpProtocol