[Telepathy] Salut branches needing review
Guillaume Desmottes
guillaume.desmottes at collabora.co.uk
Thu Feb 5 04:16:45 PST 2009
Le mercredi 04 février 2009 à 15:45 +0000, Sjoerd Simons a écrit :
> On Tue, Feb 03, 2009 at 02:40:36PM +0000, Guillaume Desmottes wrote:
> > Le mardi 03 février 2009 à 11:46 +0000, Guillaume Desmottes a écrit :
> > > ??? requestotron-tubes-with-gibber-listener: Implement stream tube new
> > > API. All Sjoerd's review complaint should be fixed
> >
> > I pushed a rebased version of it to
> > requestotron-tubes-with-gibber-listener-REBASED.
> >
> > Please review it first, if I have to rebase it again, I'll go postal. :)
>
> * salut_tubes_channel_tube_request
> - Only called by salut_tubes_manager_requestotron, both functions have
> quite a bit of overlap in the data they get from the request
I know but that's how it's done in Gabble.
How do you want to change it? I think it's good the keep the params of
this function as generic as possible.
> * salut_tubes_channel_offer_d_bus_tube
> - parameters_copied is used unitialized... Why would it be copied anyway?
fixed.
> * salut_tubes_channel_foreach
> - Would be much nicer if implement with hash table iters
done.
> * salut_tubes_manager_close_all
> - I prefer variables that are only used in a sub-block to be defined in
> that block
Me too. done.
> * salut_tubes_manager_foreach_channel
> - Hash table iteration ftw
done.
> * salut_tube_stream_constructor
> - Emitting a signal in the constructor is not usefull, nobody can have
> connected to it yet..
Good point. removed.
> * salut_tube_stream_accept_stream_tube
> - Is the FIXME still valid ?
Yeah, that's muc stream tube new API job.
G.
More information about the telepathy
mailing list