> On July 11, 2011, 8:36 p.m., David Edmundson wrote: > > main-widget.h, line 54 > > <http://git.reviewboard.kde.org/r/101918/diff/1/?file=26729#file26729line54> > > > > really try and make an effort to think if something should be public or > > not. > > > > Changing behaviour from the parent (which I assume is predicted) seems > > wrong unless you have a reason. > > David Edmundson wrote: > *protected
The reason is catching the window close event, ie. check the plasmoid upon window closing. > On July 11, 2011, 8:36 p.m., David Edmundson wrote: > > main-widget.cpp, line 1280 > > <http://git.reviewboard.kde.org/r/101918/diff/1/?file=26730#file26730line1280> > > > > If you can create something on the stack (which you can in this case) > > > > i.e "KDialog noPlasmoidDialog" > > instead of "KDialog* noPlasmoidDialog = new KDialog()" it's normally > > better to do so. > > > > It's faster, and less chance of leaking anything. Shouldn't leak if you set a parent object. But ok, stack it is. > On July 11, 2011, 8:36 p.m., David Edmundson wrote: > > main-widget.cpp, line 1284 > > <http://git.reviewboard.kde.org/r/101918/diff/1/?file=26730#file26730line1284> > > > > I think we need to brainstorm this. Nothing against that ;) > On July 11, 2011, 8:36 p.m., David Edmundson wrote: > > main-widget.cpp, line 1307 > > <http://git.reviewboard.kde.org/r/101918/diff/1/?file=26730#file26730line1307> > > > > Should it save their preference if they click "Stay Online"? I think so. Right, I thought I catched that one as well :/ > On July 11, 2011, 8:36 p.m., David Edmundson wrote: > > main-widget.cpp, line 1309 > > <http://git.reviewboard.kde.org/r/101918/diff/1/?file=26730#file26730line1309> > > > > typo in config key name. > > "clsoing" Fast typing sucks. Will fix. > On July 11, 2011, 8:36 p.m., David Edmundson wrote: > > main-widget.cpp, line 1339 > > <http://git.reviewboard.kde.org/r/101918/diff/1/?file=26730#file26730line1339> > > > > foreach (const Tp::AccountPtr &account) > > > > note the & for dealing with references, not copying the AccountPtr > > object. We might change that in other places as well. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101918/#review4612 ----------------------------------------------------------- On July 11, 2011, 7:44 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101918/ > ----------------------------------------------------------- > > (Updated July 11, 2011, 7:44 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > This is the second part of the DBus-plasmoid-check patch. It checks if the > interface exists on dbus and is valid. If not, it displays a dialog asking > user what to do (see screenshot). It can also remember the preference. Please > also check spelling/grammar of the text. > > > Diffs > ----- > > main-widget.h 313f74f > main-widget.cpp 28c2d9a > > Diff: http://git.reviewboard.kde.org/r/101918/diff > > > Testing > ------- > > Tested both with plasmoid active and not active, works as expected. > > > Screenshots > ----------- > > The quit dialog > http://git.reviewboard.kde.org/r/101918/s/194/ > > > Thanks, > > Martin > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
