[Bug 27201] implement Messages interface

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=27201


Jonny Lamb <jonny.lamb at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|patch                       |
  Status Whiteboard|                            |review-




--- Comment #2 from Jonny Lamb <jonny.lamb at collabora.co.uk>  2010-03-30 08:44:47 PST ---
I had a quick look over the code. Here are some comments to get you started:

-    def Send(self, message_type, text):
+    def _send_text_message(self, message_type, text):

I think it might have been me, but I was far too keen to use an underscore to
prefix method names, even though they are being called from other classes.
butterfly is not a library, so having do_this and do_that methods (sans
underscore) really doesn't matter. Louis-Francis' webcam work did this nicer,
for example.

I want to have a good look over butterfly (and perhaps tp-python too, but
later) and fix this all over the place, but let's not introduce it more.

Does that make sense? (I'm basically asking for
s/_send_text_message/send_text_message/g)

+        self._pending_messages2 = {}

Can this have a more useful name for this please? :-)

+        "Send a simple text message, return true if send correctly"""

Mismatched quotes and it's past tense, so s/send correctly/sent correctly/.

+    def _signal_text_sent(self, timestamp, message_type, text):
+            headers = {'message-sent' : timestamp,

Indented once too.

+    def GetPendingMessageContent(self, Message_ID, Parts):

I realise you copied these from the spec, but argument names Message_ID and
Parts are not very pythonic. Can we rename to message_id and parts instead?

+   
@dbus.service.signal('org.freedesktop.Telepathy.Channel.Interface.Messages',
signature='aa{sv}')
+    def MessageReceived(self, Message):

Why did you add the decorator?

I need to read the Messages interface again and take another look at your
branch.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.