> On July 30, 2014, 8:54 a.m., Vishesh Handa wrote: > > src/tagwidget.cpp, line 180 > > <https://git.reviewboard.kde.org/r/119543/diff/1/?file=294417#file294417line180> > > > > Any reason you've combined the too functions? I don't remember why, but > > there was a point when it split up. > > > > Also, if one really doesn't need it to be split up, then we should > > probably use a QScopedPointer instead of deleting it outself. > > Felix Eisele wrote: > i did googling yesterday and i think it is not needed. > I am not a fan of class members. you didnt see the dataflow. > We should i use a QScopedPointer? > > Thomas Braxton wrote: > Or you could just make it not a pointer and it cleans itself up. > > Felix Eisele wrote: > Good idea! > > Frank Reininghaus wrote: > No, this is a **very bad** idea. Creating the dialog on the stack as a > child of "this" and then executing its nested event loop will cause crashes > like the ones that are fixed by https://git.reviewboard.kde.org/r/118858/ . > You should use a QPointer for the dialog and delete it after calling exec() > and checking that the QPointer is not null. > > See the links that Dominik added in a comment to that review request of > mine for further info.
Since the whole purpose of this function is to get a list of tags using a dialog, the dialog must exist when it returns from exec(). Otherwise the whole function is **moot**. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119543/#review63489 ----------------------------------------------------------- On July 30, 2014, 6:05 p.m., Felix Eisele wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119543/ > ----------------------------------------------------------- > > (Updated July 30, 2014, 6:05 p.m.) > > > Review request for Baloo and Vishesh Handa. > > > Repository: baloo-widgets > > > Description > ------- > > Removed all KDialogs from balooWidgets. Removed not needed includes > > > Diffs > ----- > > src/kedittagsdialog.cpp c83ce9d > src/kedittagsdialog_p.h 0bcf744 > src/tagwidget.h 2843acd > src/tagwidget.cpp f2c3601 > src/tagwidget_p.h 045a185 > > Diff: https://git.reviewboard.kde.org/r/119543/diff/ > > > Testing > ------- > > > Thanks, > > Felix Eisele > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
