[MERGE] Mail clients listed in a registry
John Arbash Meinel
john at arbash-meinel.com
Tue Aug 19 17:29:44 BST 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Neil Martinsen-Burrell wrote:
> Here is a patch to make mail clients extensible by plugins by listing
> them in a registry. While Bazaar may reasonably try to support all
> popular mail clients, there will always be niche programs and platforms
> that we can't support. There will also be times where some vaporware
> exists to support a certain client, but it hasn't landed yet. (This is
> currently the case with Mail.app.)
>
> The patch makes it possible to add a mail client from a plugin as the
> attached plugin demonstrates. (This email sent using Mail.app and that
> plugin.) Test coverage is still adequate as we currently test the
> getting of mail_clients from Config objects for every built-in client.
>
> -Neil
>
Hmm.. weird.
It seems your __init__.py script was shown inline, but your bzr patch was not.
I started reviewing it, thinking it was the patch. It might have been a
little better to do 2 separate emails. (__init__.py doesn't really conform to
some of our coding standards, but it doesn't really matter as it is *your*
plugin.)
+from bzrlib.mail_client import mail_client_registry
""")
^- I really prefer it if lazy_import is only done on module objects. So
instead do:
from bzrlib import mail_client
...
mail_client.mail_client_registry.get(selected_client)
(If there is already a "from bzrlib import" then mail_client should be
inserted into that list.)
+mail_client_registry.register_lazy(
+ 'editor',
+ 'bzrlib.mail_client',
+ 'Editor')
^- There is no reason to use "register_lazy" as you have declared the class
*right there*. 'register_lazy' is for when the declared class may not need to
be imported at all.
For example, if you defined the mail_client_registry in config.py and then
used 'register_lazy' on everything.
In this particular case, I'm happier seeing 'mail_client_registry' in
mail_client.py and having the mail_client.py module not get loaded until you
need it.
So these should generally become:
mail_client_registry.register('editor', Editor)
Eventually it might be nice to add help strings, as then we can auto-generate
documentation for them. But that is certainly outside the scope of your change.
BB:tweak
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIqvT4JdeBCYSNAAMRAs0TAJwJgDfFQ5BmW8e9CHat99HKQeoCDgCeJnwi
q/72K37zb6lTJg2amRy8WUA=
=h2Q1
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list