Created attachment 21430 [details] [review] the patch This patch implement the whole Connection.Interface.SimplePresence interface for Telepathy-butterfly
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>
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.