Code review comment for lp:~jonathanjgster/ibid/jjg

Revision history for this message
Michael Gorven (mgorven) wrote :

On Wednesday 25 March 2009 14:14:36 Stefano Rivera wrote:
> I approve this conditional on e.message being changed to unicode(e) during
> merge.

The message shouldn't be explicitly logged at all -- log.exception() will do
that automatically.

You also can't rely on the source name (the Jabber source could be called
anything). Instead of "event.source == 'jabber'" you should
use "ibid.sources[event.source].type == 'jabber'".

I approve if these two things are changed.
 review approve

review: Approve

« Back to merge proposal