> On July 6, 2011, 9:42 a.m., Martin Klapetek wrote: > > app/chat-window.cpp, line 107 > > <http://git.reviewboard.kde.org/r/101862/diff/1/?file=26130#file26130line107> > > > > This can be null action if user presses Esc for example. So add a check > > for this pointer.
I don't think a NULL pointer would do any harm. It's only accessed inside if blocks. > On July 6, 2011, 9:42 a.m., Martin Klapetek wrote: > > app/chat-window.cpp, line 135 > > <http://git.reviewboard.kde.org/r/101862/diff/1/?file=26130#file26130line135> > > > > Please take this comment out of the for loop ;) I uncommented it, since It seems to be useful. > On July 6, 2011, 9:42 a.m., Martin Klapetek wrote: > > app/chat-window.cpp, line 154 > > <http://git.reviewboard.kde.org/r/101862/diff/1/?file=26130#file26130line154> > > > > Watch for coding style - void ChatWindow::removeTab(ChatTab* tab) Done. Although I'll probably have a separate commit later just fixing code style issues. > On July 6, 2011, 9:42 a.m., Martin Klapetek wrote: > > app/chat-window.cpp, line 189 > > <http://git.reviewboard.kde.org/r/101862/diff/1/?file=26130#file26130line189> > > > > Shouldn't you delete the 'tab' here instead of chatWidget? > > David Edmundson wrote: > this is deleting the tab, just variable is named wrong. > Also (and I know this isn't your code) it should be > chatWidget->deleteLater(); Done. And also, I noticed a destroyed signal in the docs while I was reading up on that. Should I replace aboutToClose() with QObject::destroyed ? > On July 6, 2011, 9:42 a.m., Martin Klapetek wrote: > > app/telepathy-chat-ui.h, line 46 > > <http://git.reviewboard.kde.org/r/101862/diff/1/?file=26131#file26131line46> > > > > Some whitespace leftover I'll fix these up later. - Lasath ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101862/#review4439 ----------------------------------------------------------- On July 6, 2011, 5:55 p.m., Lasath Fernando wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101862/ > ----------------------------------------------------------- > > (Updated July 6, 2011, 5:55 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > Okay, internet's really sketchy here - I don't have time to type a long > description. > > I'm not sure I handled the result from KMenu::exec() right, but apart form > that my changes *should* be fairly straightforward in the diff. > > http://quickgit.kde.org/?p=clones%2Ftelepathy-chat-handler%2Ffernando%2FdetachableTabs.git&a=shortlog&h=refs/heads/refractored_tabs > > > Diffs > ----- > > app/chat-tab.h 2175fe2 > app/chat-tab.cpp 23c912e > app/chat-window.h 2b2b70d > app/chat-window.cpp 517b694 > app/telepathy-chat-ui.h 306912d > app/telepathy-chat-ui.cpp 1654a9d > > Diff: http://git.reviewboard.kde.org/r/101862/diff > > > Testing > ------- > > Detached a few conversations with friends. > > > Thanks, > > Lasath > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
