----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103481/#review9127 -----------------------------------------------------------
Close enough. There is just one big issue and some annoyances (but hey that's me so you'd better expect that) contact-list-widget.h <http://git.reviewboard.kde.org/r/103481/#comment7547> Code style: declare after slots contact-list-widget.h <http://git.reviewboard.kde.org/r/103481/#comment7548> Code style: put right before Q_OBJECT (and include a Q_DISABLE_COPY while you're at it) contact-list-widget.cpp <http://git.reviewboard.kde.org/r/103481/#comment7549> NNNOOOOOOOO. When you use Q_DECLARE_PRIVATE, *never* use d_ptr directly. Instead, there is a handy macro for you: Q_D. Use it like that: Q_D(ContactListWidget); at the beginning of each function where you need to access the d pointer. In fact, this macro generates a "d" variable which is your dpointer with the correct type. This allows you to subclass private classes. So, once you use Q_D, you can switch all of your d_ptr-> to d-> contact-list-widget.cpp <http://git.reviewboard.kde.org/r/103481/#comment7550> Hmm, wouldn't KMessageBox be a better fit here? contact-list-widget.cpp <http://git.reviewboard.kde.org/r/103481/#comment7551> constify and watch for the style: Q_FOREACH (const QString &filename, filenames) contact-list-widget_p.h <http://git.reviewboard.kde.org/r/103481/#comment7552> A decent code style here wouldn't hurt. align all of the first letters, your choice whether to put the comma under the colon for each variable or not. - Dario Freddi On Dec. 20, 2011, 7:54 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103481/ > ----------------------------------------------------------- > > (Updated Dec. 20, 2011, 7:54 p.m.) > > > Review request for Telepathy. > > > Description > ------- > > This second part moves the actual view out of MainWidget to a separate > ContactListWidget class. The separate private class is mainly for the context > menu, which needs access to the private variables. The widget should be fully > self contained with public slots to change its behaviour/look (switch between > groups/accounts, show offline users etc). All error messages are sent out as > signals and picked by MainWidget, which displays the actual notifications. > All Tp stuff was also moved there. > > > This addresses bug 279107. > http://bugs.kde.org/show_bug.cgi?id=279107 > > > Diffs > ----- > > CMakeLists.txt e6d7cea > contact-list-widget.h PRE-CREATION > contact-list-widget.cpp PRE-CREATION > contact-list-widget_p.h PRE-CREATION > context-menu.h 178cdf0 > context-menu.cpp fff8c7a > main-widget.h f01b377 > main-widget.cpp 1d3e10c > main-widget.ui d71a276 > > Diff: http://git.reviewboard.kde.org/r/103481/diff/diff > > > Testing > ------- > > Tested all actions. Started chat etc. All works. > > > Thanks, > > Martin Klapetek > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
