Bug 19248

Reassign to ML

Hi. Thanks for the patch. I'm a bit of a newbie to pymsn, but here are a few things I found:

 * The URL to the SimplePresence spec just points to Simple_Contact_Presences.
 * SetPresence: the status argument might not be a key in to_pymsn and in that
   case should raise InvalidArgument.
 * I took your D-Bus property implementation, modified it a little, and merged
   it upstream into tp-python; therefore this part of the patch needs updating.
 * "TODO: Timestamp" -- please explain.

I'm planning to make a telepathy-python release soon. It will include the new DBusProperties mixin so you should update your implementation to use it (and bump the tp-python dependency).

 * "TODO: Timestamp" -- please explain.
I just forgot to remove it, the Presence spec required the time of the last activity to be included but this requirement isn't present in SimplePresence

 * The URL to the SimplePresence spec just points to Simple_Contact_Presences.
 * SetPresence: the status argument might not be a key in to_pymsn and in that
   case should raise InvalidArgument.
 * I took your D-Bus property implementation, modified it a little, and merged
   it upstream into tp-python; therefore this part of the patch needs updating.
All the above points should be fixed in my new version of the patch

Created attachment 21678 [details] [review]
new version

Thanks for the updated patch. Just three tiny comments:

 * telepathy.server.DBusProperties.__init__(self)
   This is an unnecessary addition as telepathy.server.Connection calls this in
   its constructor. To be fair, this is a little harsh on you as I told you to
   post this patch before I released tp-python.

 * "(presence_type,presence,personal_message)" needs spaces

 * There are excessive newlines between the two classes in simple_presence.py.

It's just worth noting here for my benefit that this requires an updated dep of tp-python to 0.15.4.

Thanks again for your patch.

(In reply to comment #6)

> It's just worth noting here for my benefit that this requires an updated dep of
> tp-python to 0.15.4.

Maybe start the NEWS entry for the futur release documenting the new dep? See http://git.collabora.co.uk/?p=telepathy-salut.git;a=blob;f=NEWS;h=8003db5e48b9a324aab9fe5aa3fadc6b11d1c4f9;hb=HEAD for the syntax of such files.

I made the corrections you asked and created a git branch containing the changes here : 
http://repo.or.cz/w/telepathy-butterfly.git?a=shortlog;h=refs/heads/simple-presence

Merged, thanks.

commit 66a545b62a57dc68ab2022b4ce42d84d11cf85f4
Author: Olivier Le Thanh Duong <olivier@lethanh.be>
Date:   Mon Jan 5 23:08:37 2009 +0100

    Implement simple presence

    Signed-off-by: Jonny Lamb <jonny.lamb@collabora.co.uk>