Bug 27742 - Add Account and AccountManager interfaces
Summary: Add Account and AccountManager interfaces
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-python (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Morten Mjelva
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-19 11:03 UTC by Morten Mjelva
Modified: 2010-04-22 03:32 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
adds Account and AccountManager classes (5.61 KB, patch)
2010-04-19 11:03 UTC, Morten Mjelva
Details | Splinter Review
adds Account and AccountManager classes (5.59 KB, patch)
2010-04-19 11:11 UTC, Morten Mjelva
Details | Splinter Review
Made requested changes (3.99 KB, patch)
2010-04-19 12:32 UTC, Morten Mjelva
Details | Splinter Review
adds Account and AccountManager classes (5.79 KB, patch)
2010-04-19 13:05 UTC, Morten Mjelva
Details | Splinter Review
adds Account and AccountManager classes (5.70 KB, patch)
2010-04-19 14:09 UTC, Morten Mjelva
Details | Splinter Review

Description Morten Mjelva 2010-04-19 11:03:11 UTC
Created attachment 35165 [details] [review]
adds Account and AccountManager classes

tp-python should support all interfaces in the spec. this bug aims to work towards that.
Comment 1 Morten Mjelva 2010-04-19 11:11:04 UTC
Created attachment 35166 [details] [review]
adds Account and AccountManager classes
Comment 2 Jonny Lamb 2010-04-19 12:01:55 UTC
Review of attachment 35166 [details] [review]:

You also didn't add these new files to client_PYTHON in Makefile.am.

::: src/client/account.py
@@ +25,3 @@
+from telepathy.interfaces import ACCOUNT
+
+DBUS_PROPERTIES = 'org.freedesktop.DBus.Properties'

Use dbus.PROPERTIES_IFACE instead.

@@ +33,3 @@
+            bus = dbus.Bus()
+
+        self.service_name = 'org.freedesktop.Telepathy.AccountManager'

telepathy.interfaces.ACCOUNT_MANAGER

@@ +36,3 @@
+        self.object_path = object_path
+        self._ready_handler = ready_handler
+        self.error_cb = error_handler

Keep this consistent and call it "_error_cb" instead.

@@ +46,3 @@
+                error_handler=self.error_cb)
+
+    def get_interfaces_cb(self, interfaces):

I'd prefer "_get_interfaces_cb".

::: src/client/accountmgr.py
@@ +25,3 @@
+from telepathy.interfaces import ACCOUNT_MANAGER
+
+DBUS_PROPERTIES = 'org.freedesktop.DBus.Properties'

Ditto from account.py.

@@ +33,3 @@
+            bus = dbus.Bus()
+
+        self.service_name = 'org.freedesktop.Telepathy.AccountManager'

Ditto from account.py.

@@ +36,3 @@
+        self.object_path = '/org/freedesktop/Telepathy/AccountManager'
+        self._ready_handler = ready_handler
+        self.error_cb = error_handler

Ditto from account.py.

@@ +46,3 @@
+            error_handler=self.error_cb)
+
+    def get_interfaces_cb(self, interfaces):

Ditto from account.py.
Comment 3 Jonny Lamb 2010-04-19 12:02:55 UTC
Please re-add the patch keyword when you've fixed said problems and attached a new patch.
Comment 4 Morten Mjelva 2010-04-19 12:32:32 UTC
Created attachment 35169 [details] [review]
Made requested changes
Comment 5 Morten Mjelva 2010-04-19 13:05:51 UTC
Created attachment 35170 [details] [review]
adds Account and AccountManager classes
Comment 6 Jonny Lamb 2010-04-19 13:21:06 UTC
Review of attachment 35170 [details] [review]:

Just a couple of niggles. It's basically fine and seems to work.

::: src/client/account.py
@@ +22,3 @@
+from telepathy.client.interfacefactory import (InterfaceFactory,
+                                               default_error_handler,
+                                               )

Move this closing bracket to the line before.

::: src/client/accountmgr.py
@@ +22,3 @@
+from telepathy.client.interfacefactory import (InterfaceFactory,
+                                               default_error_handler,
+                                               )

Ditto for account.py.

@@ +32,3 @@
+
+        self.service_name = ACCOUNT_MANAGER
+        self.object_path = '/' + self.service_name.replace('.', '/')

Actually, the object path is hard-coded in the spec, so we should use the '/org/free...AccountManager/' string here.
Comment 7 Morten Mjelva 2010-04-19 14:09:29 UTC
Created attachment 35171 [details] [review]
adds Account and AccountManager classes
Comment 8 Jonny Lamb 2010-04-22 03:32:51 UTC
Merged, thanks.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.