Hi Philip! Overall, great patch :), makes things quite more generic. A few comments about it:
* What about using GTypeModule and GInterfaces for the modules? you could have a single module that implements one interface for the indexer and other interface for trackerd. * Do we really have to expose TrackerIndexer? I'd rather keep that as an internal detail, and this wouldn't work as is for external modules, we'd have to install a bunch of headers first... If we went through GTypeModule, modules could trigger signals that the indexer would listen, without any need to expose it to modules. * in src/tracker-indexer/tracker-push.c (load_modules): there's no good reason for gotos if it's just used in a single place, just make that return immediately if the directory couldn't be opened. * in src/tracker-indexer/tracker-push.h: the .c file doesn't implement tracker_push_module_[create|shutdown] * in src/tracker-indexer/Makefile.am and src/trackerd/Makefile.am: No need to discern between "pushi" and "pushd", since there's no naming collision. Cheers, Carlos On mar, 2009-02-03 at 11:52 +0100, Philip Van Hoof wrote: > This is a new patch but with the indentation right for the two new files > this time > > On Tue, 2009-02-03 at 00:40 +0100, Philip Van Hoof wrote: > > Hi there, > > > > This long E-mail is also a request for a review of the attached patch, > > Martyn. > > > > You can call me crazy for working on this until 24 'O clock, but I guess > > I wanted to have it done today. > > > > Because Ivan Frade told me he likes the concept of letting applications > > push in data into Tracker for many situations, we decided to refactor > > the basis of the Evolution support that I recently finished [1]. > > > > [1] http://live.gnome.org/Evolution/Metadata (note that this is now a > > desktop neutral standard) > > > > I made this part GModule based. Meaning that you can add more such > > "Evolution like" modules. That is also the plan. For example one module > > for KMail and very different one for an RSS web grabber will be made > > very soon. Perhaps we'll make other ones for other kinds of apps too. > > > > How it works is simple, but can get a little bit confusing to read in C. > > I recommend reading this Vala example, as it does the same thing [2]. > > > > [2] http://live.gnome.org/Vala/DBusClientSamples/Waiting > > > > In Tracker a few more things got added, like the GModule support. > > > > tracker-push.c in trackerd is more or less a GModule loader and at the > > same time what the Vala example does [2] (go read it, in code it's much > > more easy to grasp than reading this gibberish explanation!): > > > > -> Do a ListNames on DBus initially and if our service is among the ones > > we want to consume, we more or less launch the GModule's > > capabilities. > > > > -> Then we start listening for NameOwnerChanged to know when we either > > need to shutdown the GModule's capabilities, or when we need to > > launch it. > > > > Use-cases: > > > > o. Evolution is not running, trackerd starts. User now starts Evolution. > > The NameOwnerChanged will detect this event and will launch the > > GModule for Evolution. > > > > o. Evolution is running, trackerd starts. ListNames will find the > > service for us and our handler will therefore launch the GModule. > > > > o. We were once registered with Evolution, we're still running. But now > > Evolution shuts down. The NameOwnerChanged will detect this event and > > will disable the capabilities of the GModule for Evolution. > > > > Now replace Evolution with KMail if you prefer the KDE desktop. Same > > story. > > > > We decided to refactor this because we want to enable Vala, Python, C#, > > Perl, D, C++, YodaYodi++#1^^ developers to develop modules. > > > > This means that it's all GObject & GModule based now. > > > > You install a GModule that has a function that returns a GObject that > > implements some methods in a directory A, and one in a directory B. > > > > Why two? Well one for the indexer and one for the daemon. The deamon is > > for your 'things' that need to be kept alive for a long time. The > > indexer is for your 'things' that will do a lot of DB transactions and > > insertions. > > > > For for example the Evolution one the deamon one proxies RDF triples to > > the indexer one for this reason. > > > > You can take a look at the existing Evolution one's Makefile.am to see > > how to deal with that, where to install it, etc. > > > > Anyway, here's the patch. Please review asap as we'll need this for > > followup tasks soon (like the RSS and the KMail support). > > > > > > Thanks! > > > _______________________________________________ > tracker-list mailing list > [email protected] > http://mail.gnome.org/mailman/listinfo/tracker-list _______________________________________________ tracker-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/tracker-list
