Review Request 125058: Cleanup and Add directories to index list

[Ovidiu-Florin BOGDAN]

> On Sept. 5, 2015, 3:12 p.m., Vishesh Handa wrote:
> > kcms/baloo/folderselectionwidget.cpp, line 306
> > <https://git.reviewboard.kde.org/r/125058/diff/1/?file=400372#file400372line306>
> >
> >     Coding style. The { should not be on the next line. Please fix all instances of this.

This is KDE (maybe Qt) specific. I wrote it like this due to force of habbit. I'll fix them all, including the ones I didn't write.


> On Sept. 5, 2015, 3:12 p.m., Vishesh Handa wrote:
> > kcms/baloo/CMakeLists.txt, line 20
> > <https://git.reviewboard.kde.org/r/125058/diff/1/?file=400369#file400369line20>
> >
> >     You seem to be using 'QStorageInfo' instead of Solid. Why?
> >     
> >     Also, if we decide that is what we want to do, it should in a separate patch. This patch is large and should be split.

It is simpler to get the desired information with QStorageInfo, and it is less error prone. The risk for errors is smaller.

Also Solid is another dependency. Qt is already there.


> On Sept. 5, 2015, 3:12 p.m., Vishesh Handa wrote:
> > kcms/baloo/configwidget.ui, line 43
> > <https://git.reviewboard.kde.org/r/125058/diff/1/?file=400370#file400370line43>
> >
> >     Moving the checkBox from the top to bottom? Why? Also, if we want this it goes in a seperate patch.
> 
> Ovidiu-Florin BOGDAN wrote:
>     I moved it from the bottom to the top.
> 
> Vishesh Handa wrote:
>     Right, bottom to top. Why?

It seems to me more logical this way. Here's the screenshot: http://i.imgur.com/RBWxutv.png


- Ovidiu-Florin


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


On Sept. 5, 2015, 2:33 p.m., Ovidiu-Florin BOGDAN wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125058/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2015, 2:33 p.m.)
> 
> 
> Review request for Baloo, Plasma, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> Added the functionality to add directories to be indexed.
> Did some cleaning up.
> 
> 
> Diffs
> -----
> 
>   kcms/baloo/CMakeLists.txt 7415289 
>   kcms/baloo/configwidget.ui 512e4a5 
>   kcms/baloo/folderselectionwidget.h 226ab45 
>   kcms/baloo/folderselectionwidget.cpp b44d111 
>   kcms/baloo/kcm.h 6ff5813 
>   kcms/baloo/kcm.cpp 27d93e2 
> 
> Diff: https://git.reviewboard.kde.org/r/125058/diff/
> 
> 
> Testing
> -------
> 
> Tested add, remove include and exclude directories.
> 
> 
> Thanks,
> 
> Ovidiu-Florin BOGDAN
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150905/2f3f736a/attachment.html>