> On Feb. 3, 2015, 9:33 p.m., David Faure wrote: > > src/currency.cpp, line 672 > > <https://git.reviewboard.kde.org/r/122183/diff/3/?file=346607#file346607line672> > > > > well then we still have a problem if the CurrencyCategory is being > > deleted before this job finishes. Can this happen? If so, then the > > destructor of the CurrencyCategory[Private] should delete the > > networkaccessmanager, to make sure that it can't emit finished later. > > Vishesh Handa wrote: > CurrencyCategory is initialized by a singleton, so meh. > > If you want, I can add it, though now I'm having second thoughts about > this, and maybe we should be going for the separate thread + blocking > approach.
OK then the crash can't happen (no event loop after deletion of singletons), so no problem. > On Feb. 3, 2015, 9:33 p.m., David Faure wrote: > > src/currency.cpp, line 674 > > <https://git.reviewboard.kde.org/r/122183/diff/3/?file=346607#file346607line674> > > > > missing error handling on the open() call [was there before this patch] > > Vishesh Handa wrote: > Was it? I don't see any error handling. Also, I'm not sure what to do in > that case. oops my comment was confusing. I meant: "missing error handling the open() call - even though I know that this problem was already there, it wasn't introduced by this change". - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122183/#review75337 ----------------------------------------------------------- On Feb. 3, 2015, 12:49 p.m., Vishesh Handa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122183/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2015, 12:49 p.m.) > > > Review request for KDE Frameworks. > > > Bugs: 340819 > https://bugs.kde.org/show_bug.cgi?id=340819 > > > Repository: kunitconversion > > > Description > ------- > > Currency: Fetch the currency file properly > > Properly run an event loop and wait for the file to be fetched. > > Also add a test to make sure currency conversion is working. > > This patch also contains - > * https://git.reviewboard.kde.org/r/122182/ > * https://git.reviewboard.kde.org/r/122181/ > * https://git.reviewboard.kde.org/r/122180/ > > This is because reviewboard refuses to upload only a part of the diff. Please > only look at currency.cpp w.r.t the EventLoop. > > > Diffs > ----- > > autotests/convertertest.h 8129a48 > autotests/convertertest.cpp ae4298e > src/currency.cpp 715233c > > Diff: https://git.reviewboard.kde.org/r/122183/diff/ > > > Testing > ------- > > Test now passes. > > > Thanks, > > Vishesh Handa > >
_______________________________________________ Kde-frameworks-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
