Bug 50563: Add Telepathy observer - Seif Lotfy <seif@lotfy.com> - 5/31/2012 Back to Bug | Your Reviews | Help
Attachment 62351: Telepathy observer patch - Seif Lotfy <seif@lotfy.com> - 5/31/2012 (View )

Show Quick Help

From: Seif Lotfy <seif@lotfy.com>
Date: Wed, 30 May 2012 15:37:03 +0200
Subject: [PATCH 1/2] Add Telepathy observer Finished Telepathy Observer
logging calls, im and file transfers
<Overall Comment>
Previous Reviews
Siegfried-Angel Gevatter Pujals <siggi.gevatter@gmail.com>
6/4/2012
-----------------------------------------------------------------
First of all, great work, I'm looking forward to seeing the results of this!
Now some comments:
- Urgh. No URI for messages.
- When the conversation is created, do you already have a message? If not, don't give Open/Close events a message, just the contact (note that IMMessage represents a single message, not a conversation).
- Some documentation wouldn't hurt (like eg. in create_text_event explaining how it initializes the event manifestation).
- Multi-user text conversations?
- All the above for audio calls.
- Why does the NFO_AUDIO object get named after the first contact?
- observe_call_channel(): please use constants TP_CALL_STATE_*
- "event_template.set_interpretation (ZG_CREATE_EVENT); if (channel.requested == false) event_template.set_manifestation (ZG_WORLD_ACTIVITY);" this is duplicated in create_call_event and observe_call_channel (plus, if/else so there's always a single assignment would be nicer).
- This comment "//TODO: Add payloads" seems old (no longer applicable)?
- I thought we'd decided to use namespaces in JSON payloads?
- Why is the list of call participants duplicated as subjects and in the JSON?
- «details_obj.set_double_member ("duation", duration);» <- typo "duation"
- If it isn't already, please document the JSON format on the wiki
- observe_ft_channel(): please use TP_FILE_TRANSFER_STATE_* instead of magic numbers.
- query_info() will block all of datahub if it's slow for some reason; please use query_info_async (you can use yield)
- "//TODO: create if else" - another obsolete comment?
Powered by Splinter

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.