[Bug 29197] Add bindings needed for libfolks' test suite

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=29197

Philip Withnall <bugzilla at tecnocode.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://git.collabora.co.uk/ |http://git.collabora.co.uk/
                   |?p=user/pwith/telepathy-gli |?p=user/pwith/telepathy-gli
                   |b;a=shortlog;h=refs/heads/t |b;a=shortlog;h=refs/heads/t
                   |est-suite-gir-bindings      |est-suite-gir-bindings2

--- Comment #2 from Philip Withnall <bugzilla at tecnocode.co.uk> 2010-07-23 03:56:37 PDT ---
(In reply to comment #1)
> (When putting things up for review, please add the patch keyword, and put the
> branch in the URL field (or attach git-format-patch output if you prefer).)

Sorry.

Updated branch here:
http://git.collabora.co.uk/?p=user/pwith/telepathy-glib;a=shortlog;h=refs/heads/test-suite-gir-bindings2

> If you're going to (skip) methods to avoid having to think about whether/how
> they should be introspected, we should have a way to keep track of which ones
> are deliberately (skip)'d, and which ones we just haven't thought about yet. I
> suggest opening one bug for each class visible to introspection.am that's
> partly (or not at all) annotated.
> 
> For instance, the properties on TpBaseConnection should be pretty
> uncontroversial.

Bugs should be filed for the following classes/interfaces then:
 * TpBaseConnection (base-connection.h)
 * TpHandleRepo (handle-set.h)
 * TpHandleSet (handle-set.h)

> I don't see how you can use TpBaseConnectionManager correctly without calling
> its register() method, or indeed, without knowing enough about its properties
> to construct one. Is it actually needed?

I found that one of the classes I was building for the libfolks test suite was
entirely redundant. Dropping it means that TpBaseConnectionManager and
TpBaseProtocol don't need to be added to tp-glib's Vala bindings.

> You (skip) all of TpBaseProtocol, but I'm reasonably sure it's annotated
> correctly (or at least, I wrote it after learning about annotations).
> 
> >  /**
> > - * connection_shutdown_finished_cb:
> > + * connection_shutdown_finished_cb: (skip)
> 
> This isn't API, so the right fix here would have been to change the /** to /*.
> Same for list_channel_factory_foreach_one, list_channel_manager_foreach_one.

This is all irrelevant now. I've cut the changes out of my branch.

> > - * TP_INTERNAL_CONNECTION_STATUS_NEW:
> > + * TP_INTERNAL_CONNECTION_STATUS_NEW: (skip)
> 
> This is pretty much necessary to use a TpBaseConnection correctly, so skipping
> it seems suspect.

The Connection class we're using has been pinched from
tp-glib/examples/cm/contactlist/conn.c, and goes straight to CONNECTED.

> >      <method name="destroy" c:identifier="tp_handle_set_destroy">
> >        <return-value transfer-ownership="none">
> >          <type name="none" c:type="void"/>
> >        </return-value>
> >      </method>
> 
> Should this be in the GIR at all? It's the destructor, so technically it's
> (transfer full) or (transfer into-a-big-chemical-fire) or something :-)

I didn't notice that g-ir-scanner had snuck the TpHandleSet methods into the
GIR file. They're all now (skip)ped in the updated branch.

> >       <constructor name="new_from_array" c:identifier="tp_handle_set_new_from_array">
> > ...
> >           <parameter name="array" transfer-ownership="none">
> >             <array name="GLib.Array" c:type="GArray*">
> >             </array>
> 
> With (element-type uint) a la tp_intset_to_array, this could be annotated
> correctly.

Even though this is now (skip)ped, I added the annotation anyway, so as not to
waste valuable review time. :-)

> >      <method name="peek" c:identifier="tp_handle_set_peek">
> >        <return-value transfer-ownership="full">
> >          <type name="IntSet" c:type="TpIntSet*"/>
> >        </return-value>
> >      </method>
> 
> This doesn't transfer ownership. It needs (transfer none).

Same.

> >       <method name="foreach" c:identifier="tp_handle_set_foreach">
> 
> I'd skip this in favour of peeking at the underlying TpIntSet and iterating it,
> tbh. Or, if you want it visible, please make the (scope call) explicit. The
> userdata is meant to be marked as (closure), isn't it?

(skip)ped and annotated.

> >      <method name="to_array" c:identifier="tp_handle_set_to_array">
> >        <return-value transfer-ownership="full">
> >          <array name="GLib.Array" c:type="GArray*">
> 
> This should be annotated as (element-type uint) like tp_intset_to_array().

Same.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.