[GSoC] GObjectification Summary

[Ankit Vani]

On Fri, Nov 15, 2013 at 12:59 PM, Mark Doliner <mark at kingant.net> wrote:
> - The new conversation code (in gtkconv and probably elsewhere)
> assumes that if a conversation isn't an IM then it is a chat. Whereas
> the old code always checked is_im and is_chat and did nothing if both
> of those checks were false. I think this means stuff will misbehave
> and sometimes crash if a plugin tries to extend PidginConversation to
> create a third type. Am I correct? I'm totally fine with this, but I
> thought I'd mention it in case anyone knows of a good use case where
> we want to support this.

That makes sense. I changed these conditions back to how they are in the
default branch. However, even in the original code I have found places
which assume that a conversation HAS to be either an IM or a chat.

> - Our GObjects mark as translatable the name and description strings
> for each object's properties (e.g. _("Who"), _("Who you're drawing
> with.")). That's a large number of new strings to translate. They're
> not surfaced to the user, right? I imagine they could be surfaced to
> developers in API documentation? Do we think it's worth the burden on
> translators to translate these strings?

Yeah they're not surfaced to the user. It's just something I adopted
from the old gobjectification branch and then stuck to.

I've unmarked them to non-translatable.

> - Why have an unused PROP_0 enum value?

It's again something that I saw in the old gobjectification branch, and
stuck to. It's also a convention used by glib. I've seen most projects
start with PROP_0 and a very few with PROP_XXX = 1.

There is also PROP_LAST at the end of the enum lists, which is also
unused right now, but would be used if we decide to change
g_object_notify() to g_object_notify_by_pspec() in the future, which is
faster (but requires a higher glib version).

> - In what circumstances do you call g_object_notify() in a
> purple_foo_set_bar() function? Are there times when it should not be
> called? For example, why isn't it called in
> purple_whiteboard_set_prpl_ops().

In case of purple_whiteboard_set_prpl_ops, prpl_ops isn't a property.
There were some other places where I didn't have g_object_notify, which
I've added now.

You should be able to connect a callback to a change in any property of
any GObject.

> - Small bit of #if 0'ed code in dbus-server.c
> purple_dbus_message_append_values, also dealing with outgoing values,
> as with the other code I mentioned earlier. Is this code also not
> important?

Yeah, same thing. That bit of code converted outgoing values to
pointers. But since outgoing values are G_TYPE_POINTER now anyway,
that bit is not necessary.

> - Struct member comments should be copied over from old
> PurpleCircBuffer to new PurpleCircularBufferPrivate.

Done. Circular buffers was the first thing I worked on (and mostly just
took code from the old gobjectification branch) and had missed a few
things :P

> - New functions at bottom of circularbuffer.h need doc comments.

Ah yes. Done :)

> - I'm seeing a crash when trying to sign on with an AIM account. I
> haven't had a chance to try to debug it yet, but it is reproducible
> for me. Does that sound familiar? Stack trace:
> http://pastebin.com/20iw9YG0

Fixed -- I had PURPLE_CIRCULAR_BUFFER_CLASS instead of
PURPLE_CIRCULAR_BUFFER_GET_CLASS in the definition of
purple_circular_buffer_mark_read.

> I think this is fine for now. We should add a comment to these files
> near the #include "internal.h" along the lines of "We're including
> libpurple's internal.h file here because it's convenient for us. This
> file is for internal libpurple use only. It should not be included by
> 3rd party plugins."

Added this:

/* When writing a third-party plugin, do not include libpurple's internal.h
 * included below. This file is for internal libpurple use only. We're including
 * it here for our own convenience. */