----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125872/#review87689 -----------------------------------------------------------
In line 160-164, I will suggest you to not completely remove those lines(Memory leaks maybe?) but instead revert it to the way it was before I edited it(I don't remember why I changed it). That is, put d->worker->deleteLater(); there. This shold work correctly(I haven't tested it) because (d->worker) is an object derived from QObject. We will have a problem if we do this in line 234-235 as ThreadWeaver::Job isn't a QObject. But it is important to delete that Job object to prevent a memory leak here. The done signal indicates that the Job object isn't going to be used anymore and which means that, that object should be deleted. - Aroonav Mishra On Oct. 29, 2015, 5:12 p.m., rishabh gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125872/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2015, 5:12 p.m.) > > > Review request for Amarok, Stefan Derkits and Olivier Churlaud. > > > Repository: amarok > > > Description > ------- > > removed the code that was deleting the threadeweaver::job .it's a > qsharedpointer so it shouldn't be deleted manaully > > > Diffs > ----- > > src/core-impl/collections/db/sql/SqlQueryMaker.cpp c73a551 > > Diff: https://git.reviewboard.kde.org/r/125872/diff/ > > > Testing > ------- > > builds 100% > testing done > > > Thanks, > > rishabh gupta > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
