Le mercredi 19 février 2014 à 02:00 +0100, Carlos Garnacho a écrit : > Hey,
> So from looking at the code, the implementation looks quite correct, and > it looks like a positive addition, I just have minor improvements: \o/ > 0. Nice cleanup to tracker-extract's tracker-config.c, long time > overdue :) Thanks. I think it is the right way of doing it. I copied what tracker-miner-fs does. But Martyn wasn't totally sure in his comment: https://bugzilla.gnome.org/show_bug.cgi?id=719802#c26. Do you confirm it's good? > 1. tracker_decorator_set_priority_rdf_types() is added, but there's no > matching GObject property, would be great to get one. Hm. I don't keep the strv of rdf types, only a GArray of their ids, so a getter is a bit more complex -but not impossible- to write. I'll do that if you think it's important. > 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. Right. If I understand correctly that code path is only hit on testing purpose. So I just added a quick sync query. > 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. Ok, done that. > Also, that function does if (..) if (...), when those are mutually > exclusive, would seem clearer with an if (...) else if (...) Done. > 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 Fixed. > 5. In tracker-extract-decorator.c:578: > goto out; > } > out: > > That goto feels a bit pointless :) I done that in case we ever add something else inbetween. I'm pretty sure the compiler is smart enough to optimize out a jmp instruction. Anyway, removed it if you think it's better :) > And that's all I could spot in that branch, nice work Xavier :) Thanks! Branch updated, top 4 commits fixes each of your comments: http://cgit.collabora.com/git/user/xclaesse/tracker.git/log/?h=priority Regards, Xavier Claessens. _______________________________________________ tracker-list mailing list tracker-list@gnome.org https://mail.gnome.org/mailman/listinfo/tracker-list
