On 02/17/2011 01:21 PM, Martin Langhoff wrote:
On Wed, Feb 16, 2011 at 12:54 PM, Simon Schampijer<si...@schampijer.de> wrote:
the general approach - just a few small comments inline.
Thanks! There's a patch in reply, and a few comments from me here...
+ def remove_window_by_xid(self, xid):
+ """Remove a window from the windows stack."""
+ for wnd in self._windows:
+ if wnd.get_xid() == xid:
+ # must break after changing array
I think this comment can go away. The code itself should be clear enough.
Sugar developers are smarter than I expect then :-) My concern is
about someone "expanding" the code inside the loop, and getting bitten
by my doing something that is not quite kosher.
I think this code is ok without the comment :) Actually there is another
occasion of the same code that has no comment - if we think it is
necessary we should have it in both.
@@ -510,11 +529,12 @@ class ShellModel(gobject.GObject):
home_activity = Activity(activity_info, activity_id,
window)
self._add_activity(home_activity)
else:
- home_activity.set_window(window)
+ logging.debug('setting new window for existing activity')
+ home_activity.add_window(window)
This code gets called for each window including the launcher. To make that
clear maybe you can come up with another message.
I tried to make things clearer with a comment block for the function,
and more explicit debug lines. HTH.
Yes, looks good now.
logging.debug('%s launched in %f seconds.',
home_activity.get_type(), startup_time)
With the current logic the debug log does not log what we expect,
Here is where we were crashing in race conditions :-/ Anyway, I had
that as a separate patch (posted together w this one). Now I've merged
this change in the patch too.
Thanks - looks good now.
Regards,
Simon
_______________________________________________
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel
