GNOME Bugzilla – Bug 609454
Style for <delegate>.destroy() methods
Last modified: 2010-02-20 17:05:52 UTC
<owen> Do we have a style for whether delegate's destroy() methods should just be this.actor.destroy() and be combined with a signal connection to destroy, or whether it's OK to do cleanups in the delegate destroy method? <danw> i would have said that we do have a style, and it's the former <walters> i don't think we have a consistent style but i much prefer the former --- Possible violations of the style: lightbox.js:Lightbox.destroy This is actually a signal handler and should be called _onDestroy rather than destroy(). workspace.js:Workspace.destroy workspacesView.js:GenericWorkspacesView.destroy These methods do a lot of stuff; there might be some subtle reasons that the guts can't be moved to an _onDestroy(), but not obviously. sidebar.js:Sidebar.destroy widgetBox.js:WidgetBox.destroy widget.js:ClockWidget.destroy Intent might be to have the widgets destroyed before the actor hierarchy. This is a little outside or normal scope. As well as fixing up LightBox/Workspace/GenericWorkspace, the other thing to close this bug would be adding a note about this to http://live.gnome.org/GnomeShell/StyleGuide.
Created attachment 154148 [details] [review] Use _onDestroy() handlers for cleanup in delegates Unify the style of <delegate>.destroy() methods to only contain a call to <delegate>.actor.destroy() and handle additional cleanup in a _onDestroy() signal handler.
Review of attachment 154148 [details] [review]: Looks reasonable to me. Note my note about StyleGuide before closing the bug.
Comment on attachment 154148 [details] [review] Use _onDestroy() handlers for cleanup in delegates Attachment 154148 [details] pushed as 9f43ed3 - Use _onDestroy() handlers for cleanup in delegates Not closing because of the missing addition to the style guide.
updated style guide