[Telepathy] Empathy File Transfer review
Xavier Claessens
xclaesse at gmail.com
Sun Dec 30 14:12:55 PST 2007
Hello,
I began to review empathy FT branch [1]. Here is some random comments
(mainly for barisione):
- extensions dir should be generated using new tp-glib stuff. See my
tpGlibClient branch on my git repos [2].
- empathy-ft-enums.h: Shouldn't that be generated by libtp/tp-glib ?
- it should depend on glib >= 2.15.0 instead of gio. Or maybe it could
check for glib >= 2.15 and fallback to gio if API is the same in both.
- in EmpathyTpFT's constructor dbus_g_object_register_marshaller and
dbus_g_proxy_add_signal calls are (or should be) done by generated code.
- Coding style: Even for one instruction after a if statement I add
brackets.
if (cond) {
stuff;
}
- get_time_msec() why not using API from empathy-time.h ?
I'm not quiet sure about the separation between EmpathyTpFT and
EmpathyFT. It seems EmpathyFT does nothing, it's just a wrapper around
EmpathyTpFT functions. I didn't read the code in details but as I
understand, one EmpathyTpFT object represents one FT channel which can
contain many ongoing FTs. So I support each running FT are represented
by one EmpathyFT object... so what's FT struct in EmpathyTpFT? It seems
there is a circular depend between EmpathyTpFT and EmpathyFT. I have to
read more in details the spec and implementation to have a better idea
on that, but I have the intuition there is a problem here.
What I imagined first was something like that:
- EmpathyTpFT does all the Telepathy calls and creates EmpathyFT
objects.
- Instead of things like empathy_ft_accept(EmpathyFT*) that calls
empathy_tp_ft_accept(EmpathyTpFT*,guint ft_id), I imagined something
like empathy_tp_ft_accept(EmpathyTpFT*, EmpathyFT*). Like that EmpathyFT
don't uses EmpathyTpFT API, that solves the circular dep.
So EmpathyTpFT does the Telepathy stuff and EmpathyFT does the IO stuff.
I'll try to do a more complete review later when I get a better idea of
how that works.
Xavier Claessens.
[1] git://git.collabora.co.uk/git/user/bari/empathy.git
[2] git://git.collabora.co.uk/git/user/xclaesse/empathy.git
More information about the Telepathy
mailing list