On Wed, Jun 04, 2008 at 11:44:09AM +0200, Tomeu Vizoso wrote:
> Looks good to me. Small comments inline:
>
> > + import logging
> > + logger = logging.getLogger('DevicesTray')
>
> What about moving the import along with the others and declaring the
> logger at the module level as in activitiesring.py?
Sure. I saw that code after I thought of the patch, but figured
someone might want the patch changes to be all in one place in the
file (just so you know why I did it that way). Changed as you
suggested.
> > + logger.warn("Not able to add icon for device [%s], because of "
> > + "an error (%s). Continuing." % (device, message))
>
> May be better to print the __repr__ of the device, with %r instead of
> %s. In some classes, __str__ won't be as good a description of the
> instance as __repr__.
Sure.
> Thanks,
>
> Tomeu
Martin
pgpLiPoXKaVrb.pgp
Description: PGP signature
_______________________________________________ Sugar mailing list [email protected] http://lists.laptop.org/listinfo/sugar
