[Tracker] Thumbnail branch review

[Martyn Russell <martyn lanedo com>]

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.

--
Regards,
Martyn