Re: [Tracker] GetAll method in the API

[Ivan Frade <ivan frade nokia com>]

Hi all,

 I have fixed all the comments from Jamie and Martyn and committed the
patch. 

 Regards,

Ivan

El mar, 30-09-2008 a las 09:00 +0100, ext Martyn Russell escribiÃ:
> Ivan Frade wrote:
> > Hi all,
> >
> >  We have found that the query "Give me all metadata you know about a
> > file" is pretty useful, and there were no direct method to do it in the
> > current tracker API.
> >
> >  I attach a patch that implements this feature in a new "GetAll" method
> > in the "Metadata" interface. Everything ok to commit it? Reviews and
> > comments are welcome.
>
> Patch looks good Ivan, I have a few comments:
>
> +     gboolean      valid = (result_set != NULL);
>
> There is no need to add parenthesis here, so they can be removed.
>
> +
> +     while (valid) {
> +
>
> Can we remove spaces between code block starts and the code starting?
> Please :)
>
> +             /* Item is a pair (property_name, value) */
> +             gchar **item = g_new0 ( gchar *, 2);
>
> Again, the space before the "gchar" shouldn't be there.
>
> +                     item [1] = g_strdup (value);
>
> Same again for the "item [1]", no space needed.
>
> +     const gchar *metadata = "SELECT MetadataID, MetadataDisplay FROM
> ServiceMetadata WHERE ServiceID = ";
> +     const gchar *keyword = "SELECT MetadataID, MetadataValue FROM
> ServiceKeywordMetadata WHERE ServiceID = ";
> +     const gchar *numeric = "SELECT MetadataID, MetadataValue FROM
> ServiceNumericMetadata WHERE ServiceID = ";
>
> I agree with Jamie about these.
>
>  TrackerDBResultSet *
>  tracker_db_metadata_get_types (TrackerDBInterface *iface,
> -                            const gchar        *class,
> -                            gboolean            writeable)
> +                            const gchar        *class,
> +                            gboolean            writeable)
>
> Note quite sure what changed here. But we should change "class" to
> another word because it breaks when building for C++. Not that we do
> that right now, but it is good practise to not use C++ keywords.
>
> Thanks for the patch Ivan! :)