Hi all,
Carlos recently worked on the extractor-controller-thread branch to add
support for cancelling extraction during umount with files on that mount
point being extracted at the time. This is the review of that branch:
http://git.gnome.org/browse/tracker/log/?h=extractor-controller-thread
--
• Shouldn't tracker_controller_initable_init() return an GError? Perhaps
chained up the g_thread_create() ?
• We don't disconnect "mount-point-removed" signal on finalize, are you
hoping the storage will only ever have 1 ref count? As a precaution I
would disconnect too.
• I would use separate new/free functions for GetMetadataData for
clarity and to avoid missing members when freeing or setting (without
g_slice_new0()).
• The tracker_controller_dbus_stop() function should set pointers to
NULL in case _start() is ever called afterwards.
• The GError should be handled properly in tracker_controller_new() with
propagation. We've recently changed the way error propagation is handled
in Tracker. See the propagation example here:
http://developer.gnome.org/glib/unstable/glib-Error-Reporting.html
• I saw this in the code:
/* FIXME: notify success from the other thread */
Shouldn't we fix that?
• I would use separate TrackerExtractTask new/free functions for the
same reasons listed above.
• Is this a typo?
+ if (!controller) {
+ if (controller) {
+ g_object_unref (controller);
+ }
• The config unref in main.c was moved from after log_shutdown to
before. IIRC, it's needed there for verbosity (or was).
• There are a few whitespace issues which I can fix up if you don't get
around to it. Philip noticed some of those too.
--
I need to review tracker-extract.c outside the diff, it's a bit munged
up with all the changes, but the rest looks fine to me.
Great work Carlos.
--
Regards,
Martyn
_______________________________________________
tracker-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/tracker-list
