> On Aug. 13, 2015, 9:27 p.m., Vishesh Handa wrote: > > src/file/extractorprocess.cpp, line 71 > > <https://git.reviewboard.kde.org/r/124699/diff/1/?file=393405#file393405line71> > > > > Could you please check if this is internally spawning an event loop? We > > want to avoid that.
>From what I understand, this should not spawn a local event loop, what it does >is checks if there is a complete line to be read at the time it is called and >returns accordingly. Though I'm not entirely sure and should probably check >the implementation. > On Aug. 13, 2015, 9:27 p.m., Vishesh Handa wrote: > > src/file/mainhub.h, line 53 > > <https://git.reviewboard.kde.org/r/124699/diff/1/?file=393410#file393410line53> > > > > I'm getting scared how `MainHub` is becoming a dumping ground for > > everything. > > > > Do these APIs need to be exposed over here. Maybe they can be exposed > > in other specific class. We're mostly just forwarding all the data + > > signals just for this dbus interface. Since this patch was huge already, I didn't do any D-Bus re-organization in this. Will be done later as I have mentioned in the intro. > On Aug. 13, 2015, 9:27 p.m., Vishesh Handa wrote: > > src/file/extractorprocess.cpp, line 79 > > <https://git.reviewboard.kde.org/r/124699/diff/1/?file=393405#file393405line79> > > > > What is this variable used for? It is for the assert in the ExtractorProcess::index method to ensure we don't write more id's to extractor's stdin until it is done processing the previous batch. > On Aug. 13, 2015, 9:27 p.m., Vishesh Handa wrote: > > tests/file/CMakeLists.txt, line 22 > > <https://git.reviewboard.kde.org/r/124699/diff/1/?file=393419#file393419line22> > > > > If you're disabling this temporarily, please add a comment why. The test needs to be completely re written with this patch, I'll add a TODO. - Pinak ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124699/#review83789 ----------------------------------------------------------- On Aug. 11, 2015, 2:55 p.m., Pinak Ahuja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124699/ > ----------------------------------------------------------- > > (Updated Aug. 11, 2015, 2:55 p.m.) > > > Review request for Baloo and Vishesh Handa. > > > Repository: baloo > > > Description > ------- > > * Previously the url beind indexed was exported by baloo_file_extractor which > was not ideal because calls to fetch currently being indexed url were blocked > till the current batch was indexed. > * Now, the extractor writes the url of the file it starts indexing to stdout, > which is read and emitted over dbus by the main thread. There's a long chain > of signals, to get the url to the mainhub right now this should be fixed in > later patches when we register different objects over dbus as discussed > during akademy. > * Also the remaining time test needs to be rewritten completely, and will > need to run baloo_file, instead of only the baloo_file_extractor. > > > Diffs > ----- > > src/dbus/CMakeLists.txt baf0cdf > src/file/extractor/app.h 9df4109 > src/file/extractor/app.cpp 2fb4fd6 > src/file/extractor/iohandler.h fac004c > src/file/extractor/iohandler.cpp 601759a > src/file/extractor/main.cpp 21df5ee > src/file/extractor/org.kde.baloo.extractor.xml 1a7c2c4 > src/file/extractorprocess.h 3106a71 > src/file/extractorprocess.cpp 7ce1936 > src/file/filecontentindexer.h 1e268c0 > src/file/filecontentindexer.cpp b8bdcdc > src/file/fileindexscheduler.h a3cb779 > src/file/fileindexscheduler.cpp 5d86d9e > src/file/mainhub.h bb4b287 > src/file/mainhub.cpp 955e23b > src/file/org.kde.baloo.file.indexer.xml 312ee7d > src/tools/baloo-monitor/CMakeLists.txt 527435d > src/tools/baloo-monitor/monitor.h 926450b > src/tools/baloo-monitor/monitor.cpp 281b7c2 > src/tools/balooctl/CMakeLists.txt 50fa50f > src/tools/balooctl/monitor.h b8e0402 > src/tools/balooctl/monitor.cpp a833f19 > tests/file/CMakeLists.txt 98f5a00 > > Diff: https://git.reviewboard.kde.org/r/124699/diff/ > > > Testing > ------- > > Monitor seems to be working as before. > > > Thanks, > > Pinak Ahuja > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
