On Thu, Sep 3, 2009 at 7:36 PM, Marco Martin<notm...@gmail.com> wrote: > this patch adds a proxy to the fade effect that permits to set ignored > windows, all the windows where slidingpopups is applied add themseves > to this list
- Does not meet coding style (Check whitespace between parentheses and end of lines as well as brace locations) - Example comments have not been removed (E.g. "// Example proxy code, TODO: Use or remove") - Typos in other comments - kDebug() should be in zone 1212 (Although I think this is no longer required in trunk it should still be added for consistency) - Those debug comments probably shouldn't even be required - FadeEffect::proxy() should not be const'ed on either side due to recent proxy API changes at your request - Code does not take into account when a window initially slides out yet has been configured to not slide back in on close (Currently it prevents the window from being faded as windows are only ever added to the list, never removed) - Memory leak: Windows are not removed from FadeEffect::ignoredWindows when they are deleted -- Lucas Murray :: http://www.undefinedfire.com GPG Fingerprint: 0B88 499E 3F5B 1405 D952 258A AD90 B4F5 90B6 3534 _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
