[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