> On Feb. 4, 2013, 1 p.m., Vishesh Handa wrote:
> > Overall this is pretty good. I'm just not too fond off the idle check for 3 
> > seconds. Maybe we have some precise started/stopped signals in the 
> > metadata-mover? That way we wouldn't need this 3 seconds timer.

Yes the timer is a bad solution, and as I found out not necessary
I have added it, because the filewatcher switches to idle/indexing all the 
time. This is not caused by the wrongly assumed indexing queue, but because
QtCreator likes to rename the CmakeList.txt file every few seconds which is 
picked up by the FileWatcher.

So I removed the timer now in favour of:
metadataUpdateStopped();
metadataUpdateStarted();

in case of 4 signals (remove and update started/stopped)


> On Feb. 4, 2013, 1 p.m., Vishesh Handa wrote:
> > services/filewatch/metadatamover.cpp, line 112
> > <http://git.reviewboard.kde.org/r/108576/diff/1/?file=109121#file109121line112>
> >
> >     Maybe we could have a removeUpdateStarted and removeUpdateFinished?

But how are we going to create the status message?

This solution allows to have an easily translateable status message indicating 
if we remove or move the metadata.
This translated string can be used by any program connected via dbus (the kcm 
and nepomuk controller)

If we add removeUpdateStarted/removeUpdateFinished as well as 
moveUpdateStarted/moveUpdateFinished we need to call different slots in 
nepomukfilewatch to set another variable that tells us what we are actually 
doing. Which again will be used in updateStatusMessage() or another slot to 
emit the status(string) message.

In order to avoid this 3 seconds timer I'll add together with the statusMessage
metadataUpdateStarted/metadataUpdateStopped to set the idle status (and thus 
set an idle status string and allow to make the dbusmethod working that checks 
what the watcher is doing)


> On Feb. 4, 2013, 1 p.m., Vishesh Handa wrote:
> > services/filewatch/metadatamover.cpp, line 119
> > <http://git.reviewboard.kde.org/r/108576/diff/1/?file=109121#file109121line119>
> >
> >     ditto

see above


> On Feb. 4, 2013, 1 p.m., Vishesh Handa wrote:
> > services/filewatch/nepomukfilewatch.h, line 84
> > <http://git.reviewboard.kde.org/r/108576/diff/1/?file=109122#file109122line84>
> >
> >     We need to find better names. We aren't really indexing anything over 
> > here. We're updating the metadata of some file.
> >     
> >     MetadataUpdateStarted?
> >     
> >     I'm not sure.

You are rigth, this isn't good. I decided to use this just to have the same 
naming sheme.

metadataUpdateStarted and metadataUpdateStopped sounds better though


> On Feb. 4, 2013, 1 p.m., Vishesh Handa wrote:
> > services/filewatch/nepomukfilewatch.h, line 100
> > <http://git.reviewboard.kde.org/r/108576/diff/1/?file=109122#file109122line100>
> >
> >     ditto

changed to isUpdatingMetaData()


> On Feb. 4, 2013, 1 p.m., Vishesh Handa wrote:
> > services/filewatch/nepomukfilewatch.cpp, line 120
> > <http://git.reviewboard.kde.org/r/108576/diff/1/?file=109123#file109123line120>
> >
> >     I don't think you need to do this. It's already done by the 
> > nepomukservicestub.
> >     
> >     I could be wrong.

You are right the nepomukservicestub does this.
Seems there is another problem though. If I remove this line, the dbusinterface 
adds the dbus methods correctly (isUpdatingMetaData() and statusMessage() )
But the signals are not exported to dbus.

I'll have a look at it


- Jörg


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


On Feb. 5, 2013, 9:01 p.m., Jörg Ehrichs wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108576/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2013, 9:01 p.m.)
> 
> 
> Review request for Nepomuk and Vishesh Handa.
> 
> 
> Description
> -------
> 
> The status of the current operation (move/remove metadata) is exported
> via dbus to show what is going on.
> 
> New dbus methods
>  * statusMessage - idle message or operation + url
>  * isUpdatingMetaData - true/false depending if something is happening
> 
> New signals
>  * matadataUpdateStarted
>  * metadataupdateStopped - only emitted if nothing happend for 5 seconds
>  * status(int status, string msg) - the integer shows idle/active state
> 
> The integer for the status signal was from akonadi_nepomuk feeder
> thus we do not need to query if the changes status message also means
> the indexer is idle/active (reduce dbus traffic)
> 
> 
> Diffs
> -----
> 
>   interfaces/org.kde.nepomuk.FileWatch.xml e2784a2 
>   services/filewatch/CMakeLists.txt 63307c0 
>   services/filewatch/metadatamover.h 6920849 
>   services/filewatch/metadatamover.cpp e771f02 
>   services/filewatch/nepomukfilewatch.h c076601 
>   services/filewatch/nepomukfilewatch.cpp ea71194 
> 
> Diff: http://git.reviewboard.kde.org/r/108576/diff/
> 
> 
> Testing
> -------
> 
> * apply patch
> * connect to signals via qdbusviewer
> * move files around
> 
> The status message is shown correctly, idle status switches correctly
> No idle status is set if many move/remove operations take place (only after 
> all operations are finished)
> 
> 
> Thanks,
> 
> Jörg Ehrichs
> 
>

_______________________________________________
Nepomuk mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/nepomuk

Reply via email to