Hi there, Fixes for these issues can be found here, let me know if the branch is ready for push to master.
http://git.gnome.org/browse/tracker/commit/?h=thumbnails&id=fd4a19f50793f7aaed0f8e7fe435b5e50abbe019 Cheers and thanks for the quick review, Martyn Philip On Mon, 2010-02-01 at 13:31 +0000, Martyn Russell wrote: > Hi all, > > Philip recently re-added some thumbnail work in a branch, this is the > review of that branch: > > http://git.gnome.org/browse/tracker/log/?h=thumbnails > > > 1. No need to add if() around these: > > + if (mime_types) > + g_strfreev (mime_types); > + > + if (uri_schemes) > + g_strfreev (uri_schemes); > > > 2. We shouldn't need these anymore either, we only use URIs. This would > speed things up a bit (there are several cases of this): > > + if (!strstr (from_uri, "://")) { > > > 3. I think this is pointless and will fail anyway AFAICS. Looking up > file information when the file is removed can't success: > > + file_info = g_file_query_info (file, > + G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE, > + G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, > + NULL, NULL); > + > + if (file_info) { > + tracker_thumbnailer_remove_add (uri, > g_file_info_get_content_type (file_info)); > + g_object_unref (file_info); > + } > + > > > 4. I am not sure this is the right way to do this. I would also make the > 50 a #define so it is easily changed in both places. Right now without > this code we already push the updates when the queues are done, this is > only problematic if we want more immediate updates (i.e. before the > miner is done). > > + if ((g_slist_length (private->moves_from) + > + g_slist_length (private->removes)) > 50) { > + tracker_thumbnailer_send (); > + } > + > > > The rest looks fine to me. I share your sentiments on the recursive > comment too. > -- Philip Van Hoof, freelance software developer home: me at pvanhoof dot be gnome: pvanhoof at gnome dot org http://pvanhoof.be/blog http://codeminded.be _______________________________________________ tracker-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/tracker-list
