[Tracker] REVIEW: Xavier Claessens' "priority" branch



Hey,

I've been having a look at xclaesse's branch to get prioritization of
certain rdf:types over others at the time of extracting metadata:

http://cgit.collabora.com/git/user/xclaesse/tracker.git/log/?h=priority

There, TrackerDecorator gains API to specify those high priority
rdf:types, which will affect how items are sorted/queued. 

tracker-extract does expose this setting for its TrackerDecorator
implementation through a new SetRdfTypes DBus method, so running apps
can register these, callers are monitored so the rdf:types return to
regular priority if those disappear.

So from looking at the code, the implementation looks quite correct, and
it looks like a positive addition, I just have minor improvements:

0. Nice cleanup to tracker-extract's tracker-config.c, long time
overdue :)

1. tracker_decorator_set_priority_rdf_types() is added, but there's no
matching GObject property, would be great to get one.

2. In tracker-decorator.c:607:
   /* FIXME: Can we know the class_name_id ? */
   element_add (decorator, subject, 0, FALSE);

   Probably the right approach there is accumulating those, and querying
those subjects similarly to how tracker_decorator_started() does.

3. In tracker-extract-controller.c:41:
   static void
   files_miner_status_changed (TrackerExtractController *self,
                               const gchar              *status)

   If we just care about status equality with "Idle", I'd make this
function take a boolean, or wrap one taking a boolean, so we don't need
to make up status strings in some calls.

   Also, that function does if (..) if (...), when those are mutually
exclusive, would seem clearer with an if (...) else if (...)

4. In tracker-extract-controller.c:190:
   self->priv->watch_id = g_bus_watch_name_on_connection (conn,
       "org.freedesktop.Tracker1.Miner.Files",
       ...)

   The arguments there aren't indented properly

5. In tracker-extract-decorator.c:578:
                goto out;
        }
   out:

   That goto feels a bit pointless :)

And that's all I could spot in that branch, nice work Xavier :)

Cheers,
  Carlos



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]