----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124919/#review84352 -----------------------------------------------------------
Looks good. The only minor thing I realized is that I'm not sure how QtDbus behaves with interfaces which appear and disappear (contentIndexer). src/file/filecontentindexer.cpp (line 49) <https://git.reviewboard.kde.org/r/124919/#comment58414> Maybe just 'fileIndexer' or some similar small word? This is quite the mouthfull. src/file/filecontentindexer.cpp (line 133) <https://git.reviewboard.kde.org/r/124919/#comment58416> This might not be thread safe. It calls batchTimings which accesses `m_batchTimeBuffer`. This variable is also being modified in the `run` method which runs in another thread. src/file/mainhub.cpp (line 46) <https://git.reviewboard.kde.org/r/124919/#comment58413> Maybe we can just use '/'? This is getting a bit wordy, and is exposing the internal class name/structure. - Vishesh Handa On Aug. 25, 2015, 11:28 a.m., Pinak Ahuja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124919/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2015, 11:28 a.m.) > > > Review request for Baloo and Vishesh Handa. > > > Repository: baloo > > > Description > ------- > > * Previously mainhub class was being used to forward D-Bus calls to relevant > objects > * Now each object that needs D-Bus communication is registered as a separate > D-Bus object and communication takes place directly. > * FileContentIndexer has been made a long lived class now to register a D-Bus > object for it. > > > Diffs > ----- > > src/dbus/CMakeLists.txt 7c37d94 > src/file/filecontentindexer.h eeaa93f > src/file/filecontentindexer.cpp 26f98a3 > src/file/fileindexscheduler.h 90c23c9 > src/file/fileindexscheduler.cpp 89587bc > src/file/indexerstate.h 2ed8ec9 > src/file/mainhub.h 08993c6 > src/file/mainhub.cpp dcfac30 > src/tools/baloo-monitor/CMakeLists.txt 1abb16a > src/tools/baloo-monitor/monitor.h 597690a > src/tools/baloo-monitor/monitor.cpp 485db87 > src/tools/balooctl/CMakeLists.txt 2823e65 > src/tools/balooctl/main.cpp 81bedf6 > src/tools/balooctl/monitor.h 2a39ba9 > src/tools/balooctl/monitor.cpp c5d3b50 > > Diff: https://git.reviewboard.kde.org/r/124919/diff/ > > > Testing > ------- > > baloo-monitor and balooctl seem to be working as before. > > > Thanks, > > Pinak Ahuja > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
