> On Aug. 25, 2015, 4:54 p.m., Vishesh Handa wrote:
> > Looks good. The only minor thing I realized is that I'm not sure how QtDbus 
> > behaves with interfaces which appear and disappear (contentIndexer).

ContentIndexer doesn't appear and disappear now, it's a long lived class, with 
this patch. Handling the monitor registration with an appearing and 
disappearing class would be a pain.


> On Aug. 25, 2015, 4:54 p.m., Vishesh Handa wrote:
> > src/file/mainhub.cpp, line 53
> > <https://git.reviewboard.kde.org/r/124919/diff/1/?file=397994#file397994line53>
> >
> >     Maybe we can just use '/'? This is getting a bit wordy, and is exposing 
> > the internal class name/structure.

I'll rename the objects to something smaller.


> On Aug. 25, 2015, 4:54 p.m., Vishesh Handa wrote:
> > src/file/filecontentindexer.cpp, line 134
> > <https://git.reviewboard.kde.org/r/124919/diff/1/?file=397989#file397989line134>
> >
> >     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.

I'll need to check this out.


- Pinak


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124919/#review84352
-----------------------------------------------------------


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 <<

Reply via email to