Review Request 114992: Remove the "retrieved items" stuff from UpdateItemStatesThread and VersionControlObserver.

[Emmanuel Pescosta]

> On Jan. 14, 2014, 9:55 p.m., Frank Reininghaus wrote:
> > Thanks for that patch! I'm not extremely familiar with this code, and now I'm not really sure what the purpose of the member m_retrievedItems actually is. After going through the code, my understanding is that this member will always be set to true by the thread's run() method, unless m_itemStates is empty (because the body of the foreach loop will never be executed in this case). Therefore, it looks to me like the reason why this error message is shown by VersionControlObserver::slotThreadFinished() can only be that m_itemStates is empty (assuming that the thread's run() method is always run before the thread emits its finished() signal).
> > 
> > Or am I overlooking something? I mean, I agree that it probably makes sense to remove this member and the error message. I'm just trying to figure out what the root cause of the random error messages that you mention might be. Just to make sure that there is no other problem that we haven't seen yet, and which might get hidden by this patch.

> unless m_itemStates is empty
Yes exactly. When no m_plugin->beginRetrieval() returns true, the m_retrievedItems will be false.

> only be that m_itemStates is empty
Yes exactly.

> Or am I overlooking something?
No ;)

> what the root cause of the random error messages that you mention might be
When all beginRetrieval calls return false, this happens when:
* Version-Control-Plugins could not determine any item states (corrupt database, ...)
* Dropbox-Plugin, when Dropbox is not running (call plugin->itemVersion for every item and return makes no sense here)

The main problem here is, that Dolphin throws an unnecessary error message, because the user can see it when all items are "unversioned".


- Emmanuel


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


On Jan. 12, 2014, 11:59 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114992/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2014, 11:59 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Remove the "retrieved items" stuff from UpdateItemStatesThread and VersionControlObserver.
> 
> Showing an error message, makes no sense in this case, because the user can see it, when the update of version information failed.
> 
> The plugins still have the ability to show error/warning messages on real errors. (where it makes sense ;)
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/versioncontrol/updateitemstatesthread.h 2914bc2 
>   dolphin/src/views/versioncontrol/updateitemstatesthread.cpp 6be07d3 
>   dolphin/src/views/versioncontrol/versioncontrolobserver.cpp 4d939ee 
> 
> Diff: https://git.reviewboard.kde.org/r/114992/diff/
> 
> 
> Testing
> -------
> 
> Works.
> No random "Update of version infromation failed." errors in version controlled folders anymore.
> 
> With enabled dolphin-dropbox-plugin:
> No "Update of version infromation failed." when dropbox is not running.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140114/5343385f/attachment.htm>