I implemented Mail Notification in Butterfly base on draft spec described in bug 13349. Review would be appreciated.
Here is a quick review of the patch, I assume you have tested it? * Copyright : We are in 2010 now ;) * Don't use import * please * Why do you have property thae are private and then use a lambda to access them? * Please document your functions * RequestInboxURL : Function defined in other function can access variables in their parent function scope, so you shouldn't need to pass _success and so don't need the lambda * on_mailbox_new_mail_received: Please comment where this papyon event come from like it's done in the rest of butterfly + if url_data != '': + url_data += '&' * I would rather like you constructed this as a list of string and then join them + b64encode(mail_message.form_data[key]) * Please iter on form_data.items() for key, value in mail_message.form_data.items():
(In reply to comment #1) > * Copyright : We are in 2010 now ;) > * Don't use import * please Ok, see commit 111ccf69760d1ea772137c38af23ee9a112f9255 > * Why do you have property thae are private and then use a lambda to access > them? The lambda is the normal for dbus properties, the private thing is a distraction I guess, Fixed in commit 39f76463664f0b46fc0161e578a08e5a378f7649 > * Please document your functions Well it's not really a class that someone should use. This is strictly an implementation of ConnectionInterfaceMailNotification (wich is fully documented) which is used to interface papyon with DBus. But if something precise is unclear tell me, I'll put more rational. > * RequestInboxURL : Function defined in other function can access variables in > their parent function scope, so you shouldn't need to pass _success and so > don't need the lambda Ah, yes, sometimes I forget that Python is a functional language with context referencing like LISP, see commit 4aa41f04161e5441e53aefd77195cfbd86d33b98 > * on_mailbox_new_mail_received: Please comment where this papyon event come > from like it's done in the rest of butterfly > > + if url_data != '': > + url_data += '&' > * I would rather like you constructed this as a list of string and then join > them > + b64encode(mail_message.form_data[key]) > * Please iter on form_data.items() > for key, value in mail_message.form_data.items(): Ok, didn't know that one, not doing python often enough I guess, see commit 7638b2b5aca6452b1aeef2eb1013d11fccbacee7
(In reply to comment #1) > * Please document your functions > * on_mailbox_new_mail_received: Please comment where this papyon event come See commit d0f9cf29037e5272a0c196a76d4c8db58c33f996
Found this week that not all MSN client are bound to a mail account, adding two commits that make sure we don't advertise mail notification supported is MSN profile does not have EmailEnabled: 1 in it's profile. Only add MailNotification interface if profile says it's supported: http://git.collabora.co.uk/?p=user/nicolas/telepathy-butterfly.git;a=commit;h=52e9b72cbc9df31c123828081a2112b5c5658b85 MailNotification: Fixed method naming convention and added doc http://git.collabora.co.uk/?p=user/nicolas/telepathy-butterfly.git;a=commit;h=4a1723fb4fda84c185cb524775ae4325ce5c4c02
commit 354aba59a7f1a58c9e6dfa14549eaa38ff0506cc Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk> Date: Wed Jan 27 17:52:01 2010 -0500 Bumped to new version of MailNotification.DRAFT Note that this is only a spec version bump, we should implement the RequestComposeURL method.
Fully merged now, thanks to Olivier.
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.