Re: [Tracker] GetAll method in the API

[Martyn Russell <martyn imendio com>]

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! :)

-- 
Regards,
Martyn