[sugar] code reviews
Tomeu Vizoso
tomeu at tomeuvizoso.net
Tue Mar 25 05:21:58 EDT 2008
Hi Eben,
hope you don't mind if I review some code of yours ;)
* Colorize activities in Home view(s) on hover:
- icon = CanvasIcon(size=style.STANDARD_ICON_SIZE, cache=True,
+ self.icon = CanvasIcon(size=style.STANDARD_ICON_SIZE, cache=True,
If icon is only used internally, then it is private and should start
with an underscore.
+ def __icon_hovering_changed_event_cb(self, icon, event):
+ if event:
What you call event should be a boolean called hovering.
+ self._profile = get_profile()
If the profile is not part of the state of the class, then I think we
shouldn't add a reference to it like that. Why don't you call
get_profile() each time it is needed?
+ # passing the icon_size property to the Icon contructor doesn't work
+ activity_icon.props.icon_size=gtk.ICON_SIZE_LARGE_TOOLBAR
Hmm, I think Simon pushed a fix for this some days ago? You may need
to sync your sugar-toolkit tree. Mind the spaces between operators and
operands.
+ Palette.__init__(self, None, None, primary_text=activity_info.name,
Please name the parameters that is not clear what they are, like this:
Palette.__init__(self, label=None, accelerator=None,
primary_text=activity_info.name,
* Add icons to palettes of objects in Groups/Neighborhood:
- p = palette.Palette(self._model.activity.props.name)
I know it isn't your code, but please use more complete names unless
they get really big.
Great work!
Thanks,
Tomeu
More information about the Sugar
mailing list