GNOME Bugzilla – Bug 765105
Embed item_process_async inside proper functions
Last modified: 2016-06-28 16:23:32 UTC
Instead of having the BaseItem API users call the process_async function every time a photo is changed, we should make it static and embed it into the proper functions: * photos_base_item_operation_add * photos_base_item_operation_remove * photos_base_item_operations_revert
Created attachment 326136 [details] [review] embed process_async inside operation_(add|remove|revert)
Review of attachment 326136 [details] [review]: Thanks for this clean-up, Rafael. Looks good apart from some minor points: ::: src/photos-base-item.c @@ +2429,3 @@ + if (!photos_pipeline_remove (priv->pipeline, operation)) + { + g_task_return_new_error (task, PHOTOS_ERROR, 0, "Remove failed. Not a GeglNode?"); We need a 'goto out'. Otherwise the GTask will be leaked. @@ -2422,3 @@ - GTask *task; - - g_return_if_fail (PHOTOS_IS_BASE_ITEM (self)); It would be better to retain these assertions. They are cheap because they use use G_LIKELY, and if we again move this code around, we will benefit from having them.
Created attachment 330495 [details] [review] base-item: Shuffle some code around
Created attachment 330496 [details] [review] Implicitly process the pipeline during add, remove and revert
Pushed to master.