Hello all,
So Carlos recently finished the api-cleanup branch¹. I am reviewing it
here because it is important people understand the changes going on AND
to give some feedback before merging.
¹ https://git.gnome.org/browse/tracker/log/?h=api-cleanup
So comments:
1. First, thank you Carlos for clearing up the documentation aspect of
libtracker-miner. There was some left over crap which we've moved out to
libmediaart and now we should have less warnings when compiling AFAICS :)
2. The tracker-accumulators.h has:
+#ifndef __LIBTRACKER_MINER_UTILS_H__
+#define __LIBTRACKER_MINER_UTILS_H__
...
+#endif /* __LIBTRACKER_MINER_UTILS_H__ */
Which seems wrong, given it's in libtracker-common? :)
3. The tracker-crawler.[ch] looks like a lift and shift to
libtracker-common, but I wonder why it's moved out of libtracker-miner.
It seems certainly to be a feature you would use from libtracker-miner?
What I don't understand is why the Makefile.am on master shows the
crawler sources as a separate variable to the other sources. Maybe
that's why you moved it? I would think it makes more sense to keep it in
libtracker-miner even if it was private and just keep the header
internal to that library? Logically, those functions are not useful
anywhere else in the code base and I would rather not bloat
libtracker-common.
4. The tracker-storage.[ch] move made me laugh. This poor bastard has
been moved about, copied and pasted a bunch of times and there is even a
copy of it in libmediaart :) I think it was in libtracker-miner because
we need to know about storage for file system based miners and it makes
sense logically. Just wondering why you moved it out of
libtracker-miner? Saying that it looks like it was private anyway in
libtracker-miner. So moving it makes little sense and logically it
should be in libtracker-miner, given it's used for mining. No other
modules use that code IIRC, that's why we moved it out of
libtracker-common some years back.
5. I like the rename to tracker-miner-online.[ch] from
tracker-miner-web.[ch]. One concern I have is that it makes things
simpler (which is good), but also seems to remove any capability for
online services. Previously with miners like the Flickr miner, we needed
a way to authenticate and get an OK from the end user to index that
data. What we have in master currently is a basic version of the GOA
stuff that has come on leaps and bounds AFAICS. So are you assuming
those miners using the old API will need to figure all that out
themselves now? I should add, I think the use of this API is pretty
limited to one miner right now, maybe 2?
6. I think moving tracker-miner-manager.h is possibly a mistake. It's
used by tracker-control and I have a feeling that others are making use
of that API (outside the community).
7. Nice catch with tracker-writeback/ if we really were getting the
MinerManager and doing nothing with it :)
****
So in general, I like the changes, I think some of the removals (like
password provider and some of the authentication web miner stuff) really
only affect one miner or so, most don't need that framework.
However, the miner-manager removal does concern me because there is no
way for people interested in the indexing processes to get an overall
state or idea of what is going on now without checking each miner. Also,
there is no way to control ALL miners collectively which is quite
important for some systems (IIRC Nokia paused all miners when a call
came in or was made). It seems there is no way to do this now easily.
Other than that, great work Carlos! :)
--
Regards,
Martyn
Founder & Director @ Lanedo GmbH.
http://www.linkedin.com/in/martynrussell
_______________________________________________
tracker-list mailing list
tracker-list@gnome.org
https://mail.gnome.org/mailman/listinfo/tracker-list