[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

Jonny Lamb <jonny.lamb at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://cgit.freedesktop.org |http://cgit.freedesktop.org
                   |/~jonny/telepathy-glib/log/ |/~jonny/telepathy-glib/log/
                   |?h=future-account-47100     |?h=future-master-47100

--- Comment #13 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2012-05-07 11:10:29 PDT ---
(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.

Anyway, I rebased it onto master for you.

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

OK, done.

> You should add it to telepathy-glib/introspection.am

Done.

> +  gboolean dispose_has_run;
> so old school.

DATS HOW I ROLL YO

…removed.

> tp_future_account_new should be annotated with (transfer full).

Le done. That's French for done.

> http://cgit.freedesktop.org/~jonny/telepathy-glib/commit/?h=future-account-47100&id=c5405121ef1fb6a9ad5b76942cc58e624b348121
> 
> 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.

> +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.

0. http://telepathy.freedesktop.org/wiki/Style?action=diff&rev1=22&rev2=23

> priv->account_manager is not unreffed.

Good call, fixed.

> 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?

> http://cgit.freedesktop.org/~jonny/telepathy-glib/commit/?h=future-account-47100&id=50e42faecbbb44552a9d9e04c0b809158d44717c
> 
> tp_future_account_constructed: the chain_up should be done first.

Yes, done.

> Shouldn't TpFutureAccount:parameters: be a GVariant?
> Ditto 'properties'?

Fair point. Done.

> http://cgit.freedesktop.org/~jonny/telepathy-glib/commit/?h=future-account-47100&id=8f4698dc0c8ebd898d4f2bbc97476f5aab5dd5e2
> 
> +  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.

> tp_future_account_create_account_finish's doc says that only CORE has bit
> prepared but actually we get the features from the factory (which is the right
> thing to do).

I fixed the docs.

> http://cgit.freedesktop.org/~jonny/telepathy-glib/commit/?h=future-account-47100&id=64a4af6420377b9de8e277676fd12a1aa2875777
> + * tp_future_account_set_autmatic_presence:
> typo

Fixed.

> 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.

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

Yes, good point. Fixed.

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