I started in http://cgit.collabora.com/git/user/cassidy/telepathy-gabble/log/?h=next but it's very far from being finished. I'll continue on Monday.

http://cgit.collabora.com/git/user/cassidy/telepathy-gabble/log/?h=next is now ready for review

Ship it, and please tag it as 0.99.2 (then one or other of us can get on with the giant sed invocation to have connection_interface_aliasing1, etc., in generated C, for 0.99.3).

A couple of bits of cleanup which do not block merge:

> - if (!tp_handles_are_valid (handle_repo, contact_handles, FALSE, error))
> + if (!tp_handles_are_valid (handle_repo, contact_handles, FALSE, NULL))
>     return NULL;

This is fine, but you could just g_return_val_if_fail, I think (please check in telepathy-glib first but I'm pretty sure it guarantees not to call this with bad handles).

> # pointless backwards compat section
> print >> f, '[ConnectionManager]'
>- print >> f, 'BusName=org.freedesktop.Telepathy.ConnectionManager.' + manager
>- print >> f, 'ObjectPath=/org/freedesktop/Telepathy/ConnectionManager/' + manager
>+ print >> f, 'BusName=im.telepathy1.ConnectionManager.' + manager
>+ print >> f, 'ObjectPath=/im/telepathy1/ConnectionManager/' + manager

We can delete this :-)

(In reply to comment #3)
> Ship it, and please tag it as 0.99.2 (then one or other of us can get on
> with the giant sed invocation to have connection_interface_aliasing1, etc.,
> in generated C, for 0.99.3).

Done.

> > - if (!tp_handles_are_valid (handle_repo, contact_handles, FALSE, error))
> > + if (!tp_handles_are_valid (handle_repo, contact_handles, FALSE, NULL))
> >     return NULL;
> 
> This is fine, but you could just g_return_val_if_fail, I think (please check
> in telepathy-glib first but I'm pretty sure it guarantees not to call this
> with bad handles).

Indeed, it does check it.

> > # pointless backwards compat section
> > print >> f, '[ConnectionManager]'
> >- print >> f, 'BusName=org.freedesktop.Telepathy.ConnectionManager.' + manager
> >- print >> f, 'ObjectPath=/org/freedesktop/Telepathy/ConnectionManager/' + manager
> >+ print >> f, 'BusName=im.telepathy1.ConnectionManager.' + manager
> >+ print >> f, 'ObjectPath=/im/telepathy1/ConnectionManager/' + manager
> 
> We can delete this :-)

You mean we can remove the whole manager-file.py? Gabble doesn't seem to use it.
According to the comment, the master copy is in tp-glib. Should we drop it there as well?

Created attachment 87740 [details] [review]
construct_contact_statuses_cb: assert that handles are valid

(In reply to comment #4)
> > > # pointless backwards compat section
> > > print >> f, '[ConnectionManager]'
> > >- print >> f, 'BusName=org.freedesktop.Telepathy.ConnectionManager.' + manager
> > >- print >> f, 'ObjectPath=/org/freedesktop/Telepathy/ConnectionManager/' + manager
> > >+ print >> f, 'BusName=im.telepathy1.ConnectionManager.' + manager
> > >+ print >> f, 'ObjectPath=/im/telepathy1/ConnectionManager/' + manager
> > 
> > We can delete this :-)
> 
> You mean we can remove the whole manager-file.py? Gabble doesn't seem to use
> it.

I meant the BusName and ObjectPath keys in the [ConnectionManager] group, which are labelled "pointless backwards compat section" in the code I quoted :-)

> According to the comment, the master copy is in tp-glib. Should we drop it
> there as well?

Delete it from anywhere that doesn't use it? I think telepathy-glib uses it to generate .manager files for the examples, so it will probably need to keep it.

The idea of using manager-file.py instead of write-mgr-file is that it's more cross-compilation-friendly; but tbh we should probably just have a static .manager file in the source tree, compile it in with GResource, and have a TpBaseProtocol subclass that uses a GResource as its source data. Or something...

Comment on attachment 87740 [details] [review]
construct_contact_statuses_cb: assert that handles are valid

Review of attachment 87740 [details] [review]:
-----------------------------------------------------------------

++

(In reply to comment #7)
> Comment on attachment 87740 [details] [review] [review]
> construct_contact_statuses_cb: assert that handles are valid
> 
> Review of attachment 87740 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ++

merged.

(In reply to comment #6)
> (In reply to comment #4)
> > > > # pointless backwards compat section
> > > > print >> f, '[ConnectionManager]'
> > > >- print >> f, 'BusName=org.freedesktop.Telepathy.ConnectionManager.' + manager
> > > >- print >> f, 'ObjectPath=/org/freedesktop/Telepathy/ConnectionManager/' + manager
> > > >+ print >> f, 'BusName=im.telepathy1.ConnectionManager.' + manager
> > > >+ print >> f, 'ObjectPath=/im/telepathy1/ConnectionManager/' + manager
> > > 
> > > We can delete this :-)
> > 
> > You mean we can remove the whole manager-file.py? Gabble doesn't seem to use
> > it.
> 
> I meant the BusName and ObjectPath keys in the [ConnectionManager] group,
> which are labelled "pointless backwards compat section" in the code I quoted
> :-)
> 
> > According to the comment, the master copy is in tp-glib. Should we drop it
> > there as well?
> 
> Delete it from anywhere that doesn't use it? I think telepathy-glib uses it
> to generate .manager files for the examples, so it will probably need to
> keep it.

Ok I just removed it from Gabble next then.