[Bug 30478] Add TP_ACCOUNT_FEATURE_STORAGE to TpAccount

[bugzilla-daemon at freedesktop.org]

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

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
  Status Whiteboard|                            |review-, minor changes
         AssignedTo|telepathy-bugs at lists.freede |danielle.madeley at collabora.
                   |sktop.org                   |co.uk

--- Comment #3 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-10-04 04:49:53 PDT ---
Generally looks good. Some minor things:

> +  if (self->priv->storage_provider != NULL &&
> +      strlen (self->priv->storage_provider) > 0)

That use of strlen is an inefficient way to spell (s->p->s[0] != '\0'). Even
better, you could replace the entire condition with a call to the
tp_str_empty() macro.

> +  self->priv->storage_provider = g_strdup (tp_asv_get_string (properties,
> +        "StorageProvider"));

I think I'd prefer it if you set s->p->s to "" if invalid/omitted/etc.

Is it useful to distinguish between "" and NULL at all? I can see two obvious
alternatives:

* storage-provider is always a non-empty string or NULL
* storage-provider is always non-NULL but may be empty

This could either be enforced by the GetAll callback, or by the property getter
and the "C binding".

> +   * TpAccount:storage-restrictions
> ... This value will be %NULL...

No, it won't. :-)

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