[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