Bug 25995

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.