Re: [Tracker] Refactored the src/plugins & (new) Evolution support

[Martyn Russell <martyn imendio com>]

Martyn Russell wrote:
> Philip Van Hoof wrote:
> > ok, fair enough.
> >
> > http://git.codethink.co.uk/?p=tracker;a=commitdiff;h=739e33a1a7175d8f481e0fa617cc388b7b7803a7 
> >
> >
> > Was this the last remark before commit approval?
>
> I just wanted to look at the files I didn't have, 
> trackerd/tracker-push.[ch] first. Will get back to you before the end of 
> the day.

Hi Philip,

#1, instead of doing this:

+ static void
+ unload_modules (PushSupportPrivate *private)
+ {
+       GList *copy = private->modules;
+
+       while (copy) {
+               PushModule *p_module = copy->data;
+
+               p_module->shutdown (p_module->registrar);
+
+               g_object_unref (p_module->registrar);
+               g_module_close (p_module->module);
+               g_slice_free (PushModule, p_module);
+
+               copy = g_list_next (copy);
+       }
+
+       g_list_free (private->modules);
+       private->modules = NULL;
+ }


Can I suggest we either use g_list_foreach() or a for() loop here from 
tracker_push_shutdown().

#2, The load_modules() still has this if(error) goto handle_error. 
Please do not use goto for single conditions - especially when they just 
return anyway. Can you change that to just print the error in the 
condition above and return.

#3, In name_owner_changed_cb(), we have another while() loop for 
handling a list. Can we use for() there please.

#4, Again, another 2 while() cases where for() is more appropriate in 
list_names_reply_cb().

--

With those fixed, I think we can commit. I am a bit uneasy about this 
approach to "push" modules. I would much rather we had the same 
infrastructure as we do for indexer modules here. It would be expansible 
and consistent (with how we do it everywhere else already). I think you 
might find when we come to add more "push" modules that we will have to 
do something like this later down the line anyway.

Thanks again for the patch Philip :)

--
Regards,
Martyn