Review Request 111012: KFileItemModelRolesUpdater polishing, step 4: final cleanups

Commit Hook null at kde.org
Thu Jun 20 18:27:15 BST 2013


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


This review has been submitted with commit 197a4941fed83ce6748cf0e042412eaaa2346f22 by Frank Reininghaus to branch master.

- Commit Hook


On June 13, 2013, 9:27 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111012/
> -----------------------------------------------------------
> 
> (Updated June 13, 2013, 9:27 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> This depends on https://git.reviewboard.kde.org/r/111008/ and https://git.reviewboard.kde.org/r/111009/ and https://git.reviewboard.kde.org/r/111011/
> 
> To keep the other 3 patches as readable as possible, I've kept these final changes for a separate patch.
> 
> (a) With the recent changes, KFileItemModelRolesUpdater::resolveNextPendingRoles() is never called when a preview job is running -> no checks needed any more in that function.
> 
> (b) KFileItemModelRolesUpdater::applyResolvedRoles(): Always check if an icon is stored in the model already, and remove the "m_state == ResolvingSortRole || m_state == PreviewJobRunning" check. Without this change, it could happen that the sort role "type" has been resolved for the item already, i.e., the mime type and the final icon are known when we arrive here, but there is still no icon stored in the model. Hm, now that I think of it, I should maybe have put that change at the beginning of the patch series because we now have a temporary regression between steps 2 and 4.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp e539b45 
> 
> Diff: http://git.reviewboard.kde.org/r/111012/diff/
> 
> 
> Testing
> -------
> 
> Works for me.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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


More information about the kfm-devel mailing list