D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-10-01 Thread Stefan Brüns
bruns added a comment. In D15843#335011 , @smithjd wrote: > In D15843#334518 , @ngraham wrote: > > > > > > This patch (or the other) is a requirement to augment the **current implementation**

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-10-01 Thread James Smith
smithjd added a comment. In D15843#334518 , @ngraham wrote: > Perhaps we should discuss the implementation of multi-device indexing in a Phab ticket instead of across the comments of multiple patches. Then we can settle on an agreed-upon

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-30 Thread Nathaniel Graham
ngraham added a comment. Perhaps we should discuss the implementation of multi-device indexing in a Phab ticket instead of across the comments of multiple patches. Then we can settle on an agreed-upon approach and go implement it instead of going in circles with different approaches.

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-30 Thread James Smith
smithjd added a comment. >>> As a side note - cross-device indexing is broken anyway and can never work reliably in the current scheme using device ids as part of the baloo document id. Device ids are not stable. For the home partition it works somewhat reliable as the device setup is

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-30 Thread Stefan Brüns
bruns added a comment. In D15843#334515 , @smithjd wrote: > >> There is no need for this, just create a new StorageDevices where you need it. > >> Creating a second StorageDevices instance in a process is quite cheap. > > > > Creating a

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-30 Thread James Smith
smithjd added a comment. >> There is no need for this, just create a new StorageDevices where you need it. >> Creating a second StorageDevices instance in a process is quite cheap. > > Creating a separate object cluttered the console with duplicated debug output and raised a threading

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-30 Thread Stefan Brüns
bruns added a comment. As a side note - cross-device indexing is broken anyway and can never work reliably in the current scheme using device ids as part of the baloo document id. Device ids are not stable. For the home partition it works somewhat reliable as the device setup is

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-30 Thread Stefan Brüns
bruns added a comment. To cite myself from the mentioned review: > There is no need for this, just create a new StorageDevices where you need it. > Creating a second StorageDevices instance in a process is quite cheap. If you need the info in DatabaseSanitizer, create the

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-30 Thread James Smith
smithjd added a comment. In D15843#334230 , @bruns wrote: > Most obvious problem with this change - as far as I can deduce from your description, this is about runtime behaviour. The config class is the wrong place to add this method.

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-30 Thread Stefan Brüns
bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed. Most obvious problem with this change - as far as I can deduce from your description, this is about runtime behaviour. The config class is the wrong place to add this method.

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-29 Thread Nathaniel Graham
ngraham added a comment. I think what @bruns is gently trying to indicate is that your Summary needs improvement. :) Remember that it becomes a part of the commit message once the patch lands. A Test Plan wouldn't hurt, either. REPOSITORY R293 Baloo REVISION DETAIL

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-29 Thread James Smith
smithjd added a comment. In D15843#334024 , @bruns wrote: > Whats the use case exactly? "Some parts" is not sufficient ... The index cleaner for example needs to skip unmounted paths. REPOSITORY R293 Baloo REVISION DETAIL

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-29 Thread Stefan Brüns
bruns added a comment. Whats the use case exactly? "Some parts" is not sufficient ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15843 To: smithjd, #baloo, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-29 Thread Nathaniel Graham
ngraham added reviewers: Baloo, bruns. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15843 To: smithjd, #baloo, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D15843: Allow FileIndexerConfig to check device mounted status by path.

2018-09-29 Thread James Smith
smithjd created this revision. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. smithjd requested review of this revision. REVISION SUMMARY Parts of Baloo need some way to check if a path's volume is mounted or not. REPOSITORY R293 Baloo