[Bug 47100] add a way to create accounts with properties, without knowing property names/namespaces

[bugzilla-daemon at freedesktop.org]

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

--- Comment #15 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2012-05-07 23:53:46 PDT ---
(In reply to comment #13)
> (In reply to comment #7)
> > Any reason to base this on top of next? I'd like to have it in master if
> > possible so I can start using it in Empathy sooner rather than later.
> 
> I want next to come quickly and I get the impression working on master
> including working around deprecations, then having to spend more time merging
> into next and removing said deprecations is going to take longer.

I don't think it's that bad (especially with new files/API) and having it in
master makes porting non trivial app (basically Empathy) much easier.

> Anyway, I rebased it onto master for you.

Cool thanks, I'll try using it in Empathy.

> > If you do rebase, you could use the new glib-style versions macro to anotate
> > the new API.
> 
> OK, done.

Strictly speaking you should use _TP_AVAILABLE_IN_UNRELEASED but this will be
part of 0.20 so that's fine.



> > GObject properties are missing the "Since: ...".
> 
> Well, so is every other symbol? I thought Since: things were only useful for
> methods added to objects where the object has existed for a while. I added
> Since: UNRELEASED for the main TpFutureAccount header. Adding Since to each
> symbol/property makes it feel like there is one which isn't the same version as
> the others, which is wrong. What do you think? I can add them if you feel
> strongly about it.

I generally add it to all the API if only to be exhaustive and I think GLib
does as well.

> > +TpFutureAccount * tp_future_account_new (TpAccountManager *account_manager,
> > +    const gchar *manager, const gchar *protocol) G_GNUC_WARN_UNUSED_RESULT;
> > one arg per line.
> 
> huh, you changed that in 2009[0]? I don't remember that. Oh well, done.

Cool, now you just have to review and fix all the code you wrote these last 3
years.


> > http://cgit.freedesktop.org/~jonny/telepathy-glib/commit/?h=future-account-47100&id=e7c7938b89d79dedf3e99a91d145eac437cba689
> > 
> > Shouldn't tp_future_account_set_display_name() fails if the account has already
> > be created?
> > This applies to all set_*() methods as well.
> 
> Well I wondered what to do about this. You think I should make _set_ and
> _create_account be no-ops if the account has been created?

Or at least raise a warning as in tp_future_account_create_account_async().


> > +  if (priv->account_manager == NULL
> > +      || priv->cm_name == NULL
> > +      || priv->proto_name == NULL
> > +      || priv->display_name == NULL)
> > 
> > Shouldn't we check account_manager, cm_name or proto_name in constructed?
> 
> OK.

http://cgit.freedesktop.org/~jonny/telepathy-glib/commit/?h=future-master-47100&id=09ece4e16b3a8b99d92a9f48610d92949edfa732

Looks a bit weird to me. I think we should either requires users to pass the
display name when constructing the object or have a sensible default.
Empathy uses (the first where we have the data):
- $service ($param-account)
- $param-account
- $protocol account
- New account


> > I'm wondering if we shouldn't use a GVariant to store the different (presence,
> > status, message) and so reduce the number of properties.
> > 
> > http://cgit.freedesktop.org/~jonny/telepathy-glib/commit/?h=future-account-47100&id=a64f45dc0e9406771e9960ee135300cc26317131
> > 
> > We could group the 2 avatar properties as well.
> 
> I'm merely copying what TpAccount does, so the two feel more similar. I think
> keeping them consistent is a good idea.

Consistency is good indeed. I opened bug #49616 to consider changing this in
next.


> > http://cgit.freedesktop.org/~jonny/telepathy-glib/commit/?h=future-account-47100&id=a64f45dc0e9406771e9960ee135300cc26317131
> > 
> > tmp is leaked
> 
> Yes, good point. Fixed.

Did you try valgrinding the test to check for leaks?

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