Re: [PATCH] First go at empathy/bigboard integration

["Marco Pesenti Gritti" <mpgritti gmail com>]

On Mon, Jun 9, 2008 at 8:55 PM, Owen Taylor <otaylor redhat com> wrote:
> On Mon, 2008-06-09 at 20:31 +0200, Marco Pesenti Gritti wrote:
>> Hello,
>>
>> I have not yet hooked up opening chat windows because I need a
>> desktop-data-model fix to do it properly, but I would like to start to
>> get this in.
>
> Basically looks fine to commit to me. A few comments below.
>
> - Owen
>
> Index: bigboard/stocks/people/PeopleStock.py
> ===================================================================
> [....]
> +IM_CLIENT = 'empathy'
>
> This isn't used, is it?

Yeah a leftover. Fixed.

> Index: bigboard/stocks/people/imclient.py
> ===================================================================
> [...]
> +    def configure(self):
> +        dialog = empathy.configure_accounts()
> +        dialog.connect('destroy', self.__on_configure_dialog_destroy)
>
> So, this opens up the configure dialog inside the bigboard process?
> Bothers me a bit, though I can't think of any real harm from it.

Same here. It was discussed with Xavier on this bug:

http://bugzilla.gnome.org/show_bug.cgi?id=535129#c3

> +    def __on_configure_dialog_destroy(self, widget):
> +        print self.configured
> +        if self.configured:
> +            empathy.set_online(True)
> +            self.notify('configured')
>
> Left-over debug printf

Ooops, fixed.

> Index: bigboard/empathy/empathy.h
> ===================================================================
> [...]
> +gboolean   empathy_is_configured      (void);
> +GtkWidget *empathy_configure_accounts (void);
> +void       empathy_set_online         (gboolean    online);
> +void       empathy_open_chat_with     (const char *account_id,
> +                                       const char *buddy_id);
>
> Shouldn't all these function be bb_empathy_* or something like that
> to avoid tromping over the libempathy namespace?

Good point, fixed.

Thanks for the review!

Marco