Re: [Tracker] Review of tracker-writeback migration to libtracker-sparql

[Philip Van Hoof <spam pvanhoof be>]

Fixes for these issues:

http://git.gnome.org/browse/tracker/commit/?h=class-signal&id=cee62fb31d8d9a0f5aef5773247d2f3c9818c977

On Mon, 2010-08-23 at 17:10 +0100, Martyn Russell wrote:
> This is the review of commit:
>
>  
> http://git.gnome.org/browse/tracker/commit/?h=class-signal&id=a28504d1aa23cbf8957d2f1adf66694e006dad0e
>
> 1. sparql_query_cb() checks error twice: i.e.
>
>    if (!error) { .. } else { if (error != NULL) { } else { } }
>
>
> 2. For the playlist setting, you use: 
> tracker_sparql_cursor_get_string(cursor, 0), just checking that 
> shouldn't be new memory?
>
>
> 3. Shouldn't we have quotes around the artist name here?
>
> + query = g_strdup_printf ("SELECT ?artistName WHERE {<%s> 
> nmm:artistName ?artistName}",
> + urn);
>
>
> 4. Coding style, spaces please :) in writeback_taglib_get_artist_name():
>
> + g_free (query);
> + return val;
>
>
> 5. I would check the cursor not the error in the unlikely case that it 
> is unset. The cursor is what you're next operation depends on in a lot 
> of cases and that has to be non-null (especially for unrefing).
>
> --
>
> Great stuff though other than that, looks like there is less code now in 
> tracker-writeback ;)
>
> Thanks Philip,

-- 


Philip Van Hoof
freelance software developer
Codeminded BVBA - http://codeminded.be