[Nepomuk] Review Request: Added the ability to see currently indexed file and ability to pause or resume strigi in status widget

Smit Shah who828 at gmail.com
Sat Aug 20 07:02:18 UTC 2011



> On Aug. 19, 2011, 11:27 a.m., Sebastian Trueg wrote:
> > nepomuk/kcm/CMakeLists.txt, line 19
> > <http://git.reviewboard.kde.org/r/102301/diff/5/?file=32032#file32032line19>
> >
> >     Why this change?

 Well in controller this was named as servicecontrol and here the exact same thing was named as nepomukservicecontrolinterface , i thought since both do the same thing its better to have the same name.


> On Aug. 19, 2011, 11:27 a.m., Sebastian Trueg wrote:
> > nepomuk/kcm/statuswidget.h, line 57
> > <http://git.reviewboard.kde.org/r/102301/diff/5/?file=32034#file32034line57>
> >
> >     Bad name for a slot. Be more descriptive.

How about slotSuspendResume() ?


> On Aug. 19, 2011, 11:27 a.m., Sebastian Trueg wrote:
> > nepomuk/kcm/statuswidget.h, line 68
> > <http://git.reviewboard.kde.org/r/102301/diff/5/?file=32034#file32034line68>
> >
> >     AFAIK if you store pointers to the interfaces you also have to recreate them once the service comes up again (nepomuk stopped and started). I may be wrong though. Did you check whether stopping nepomuk and starting it again invalidates the status display?
> >     HINT: before you go changing all this again, TEST what I said first. Maybe I am wrong.
> >     
> >     Also m_service is not descriptive enough. Call it m_fileIndexerService instead.

 Well i tested this by enabling and disabling nepomuk few times , and after restarting it worked exactly as before so i haven't encounter any issues here but i will check it before updating the diff though and yes i will change the name.


> On Aug. 19, 2011, 11:27 a.m., Sebastian Trueg wrote:
> > nepomuk/kcm/statuswidget.cpp, line 182
> > <http://git.reviewboard.kde.org/r/102301/diff/5/?file=32035#file32035line182>
> >
> >     Code duplication with the badly names slotToggle. Instead call the toggle slot here.

Understood.


> On Aug. 19, 2011, 11:27 a.m., Sebastian Trueg wrote:
> > nepomuk/kcm/statuswidget.cpp, line 183
> > <http://git.reviewboard.kde.org/r/102301/diff/5/?file=32035#file32035line183>
> >
> >     Call it "Suspend File Indexer" and "Resume File Indexer". And if you are using i18nc be more specific in the context message like "Suspends the Nepomuk file indexing service."

 Again will change this as well.


- Smit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102301/#review5824
-----------------------------------------------------------


On Aug. 17, 2011, 1:32 p.m., Smit Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102301/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2011, 1:32 p.m.)
> 
> 
> Review request for Nepomuk.
> 
> 
> Summary
> -------
> 
> Now you can pause or resume strigi and see the currently indexed file.
> 
> 
> Diffs
> -----
> 
>   nepomuk/controller/systray.cpp 6cf6bc9 
>   nepomuk/kcm/CMakeLists.txt 561d48a 
>   nepomuk/kcm/nepomukserverkcm.cpp 74031b7 
>   nepomuk/kcm/statuswidget.h 0440745 
>   nepomuk/kcm/statuswidget.cpp 4798d7f 
>   nepomuk/kcm/statuswidget.ui 359ec6e 
> 
> Diff: http://git.reviewboard.kde.org/r/102301/diff
> 
> 
> Testing
> -------
> 
> paused and resume couple of times and it works perfectly.
> 
> 
> Screenshots
> -----------
> 
> See
>   http://git.reviewboard.kde.org/r/102301/s/223/
> see again
>   http://git.reviewboard.kde.org/r/102301/s/224/
> with current status
>   http://git.reviewboard.kde.org/r/102301/s/226/
> 
>   http://git.reviewboard.kde.org/r/102301/s/227/
> 
> 
> Thanks,
> 
> Smit
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20110820/2fc1e8d4/attachment.html>


More information about the Nepomuk mailing list