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. >> @@ -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. >> 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. cheers, m -- martin.langh...@gmail.com mar...@laptop.org -- Software Architect - OLPC - ask interesting questions - don't get distracted with shiny stuff - working code first - http://wiki.laptop.org/go/User:Martinlanghoff _______________________________________________ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel