[Tracker] Writeback branch review

[Martyn Russell <martyn lanedo com>]

Hi Philip,

I have reviewed the branch and here are my comments with the diff 
between writeback branch and master:

01, Why have tracker:writeback for nie:copyright? Are we using that?

02, Nice fix on the nfo:entryCounter maxCardinality (and others) ;)

03, Why did you disable the mp3 extractor in the Makefile.am? Same with 
totem-plparser?

04, Was this needed then? (For loop please)
+               if (module) {
+                       g_hash_table_insert (priv->modules, g_strdup 
(path), module);
+               }

05, I wonder if we are missing some content-types in the list we have 
for play lists?

06, Coding style, function params, one per line please (rewrite_playlist).

07, In func rewrite_playlist(), path is unused.

08, Why the #if 0s in that func?

09, I would not use if(!error) then if(array...), since it is likely 
that both are non null so you are checking twice for the normal 
operation, I would simply have the array check and use g_clear_error() 
(which checks for NULL GErrors anyway). Also the check for array being 
non-null is done twice for the free at the end, I would just use if 
(array) { ... if (array->len > 0) { ...} /* do free here */ }

10, Same trick with the writeback_xmp_update_file_metadata() and GErrors.

11, Why do we need to use xmp_set_property() after 
xmp_delete_property(), can't we just set?

12, For the orientation, is it right that "orientation-bottom" is 
"bottom - right", because "orientation-top" is "top - left".

13, I think the orientation and metering should be #defines, also who 
defines these? Nepomuk?

14, Extra spaces before: xmp_delete_property (xmp, NS_EXIF, 
"WhiteBalance"); and for "flash".

15, The camera make/mode code looks like it could be wrong... what if 
the model is "Canon 500D" ?

16, White space issues in line if (g_strcmp0 (row[1], TRACKER_MLO_PREFIX 
"location") == 0 ||

17, We should probably use tracker_is_blank_string() not 
tracker_is_empty_string() here.

There are other white space issues I found too, so we should really use 
git diff --check before committing here.

I can fix some of these if you would prefer.

Other than that, it looks like a good start, thanks.

--
Regards,
Martyn