Bug 50834

I say we have two options:

1. Let's use _get_protocol_name() and _get_cm_name() everywhere. I guess means updating TpConnection and obviously TpAccount.

2. Let's actually make these functions return the TpConnectionManager/TpProtocol object.

Option two makes the API nicer in my opinion, but is arguably a bit more work.

Here is branch for master: http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=cm-name

It deprecate wrong names and add the correct names. Deprecated APIs will then be removed in next.

(In reply to comment #2)
> Here is branch for master:
[...
> It deprecate wrong names and add the correct names.

That approach is perfect, IMO.

The branch looks good, except for this:

> Note that this introduce a small behaviour change: "connection-manager" and
> "protocol" properties are not CONSTRUCT_ONLY anymore. This is because we can't
> have 2 CONSTRUCT properties setting the same value, otherwise the default-value
> would override the value set on the other property. This shouldn't affect any
> application since that API was added in previous release, and
> tp_account_request_new() should be used anyway.

If you set the property twice (which is now "officially" allowed - the metadata says you can set it at any time, now!), the set_property() implementation asserts. That seems bad.

I would be inclined to omit this patch from master altogether, and just do a straight rename in next. As you say, applications will probably use new() anyway, so this shouldn't result in much/any actual porting between 0.x and 1.0.

> Deprecated APIs will then
> be removed in next.

Ideal.

(In reply to comment #1)
> 2. Let's actually make these functions return the
> TpConnectionManager/TpProtocol object.

I think that should be a separate bug, tbh.

(In reply to comment #3)
> (In reply to comment #2)
> > Note that this introduce a small behaviour change: "connection-manager" and
> > "protocol" properties are not CONSTRUCT_ONLY anymore. This is because we can't
> > have 2 CONSTRUCT properties setting the same value, otherwise the default-value
> > would override the value set on the other property. This shouldn't affect any
> > application since that API was added in previous release, and
> > tp_account_request_new() should be used anyway.
> 
> If you set the property twice (which is now "officially" allowed - the metadata
> says you can set it at any time, now!), the set_property() implementation
> asserts. That seems bad.

I don't understand what you mean. "connection-manager" is now read-only and "cm-name" is construct-only, so you can set it only once. The only issue is if an app does g_object_new (TP_TYPE_ACCOUNT_REQUEST, "connection-manager", foo, NULL); it will now fail. But only empathy uses it and any app is using tp_account_request_new() anyway. So really the issue is purely theoric.

(In reply to comment #5)
> > If you set the property twice (which is now "officially" allowed

Ah, what I'd missed was that this changed from READWRITE to READABLE, so not only are you not allowed to set it twice, you're not even allowed to set it once.

> The only issue is if
> an app does g_object_new (TP_TYPE_ACCOUNT_REQUEST, "connection-manager", foo,
> NULL); it will now fail.

That's an API break.

I would (still) be inclined to omit this patch from master altogether, and just do a straight rename in next.

ok, did like that. Branch pushed to both master and next.