[Bug 28192] support for immutable accounts in Mission Control

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Jun 18 17:49:19 CEST 2010


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

--- Comment #7 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-06-18 08:49:19 PDT ---
With a few more adjustments, this looks OK to merge when the spec lands in
telepathy-glib.

> +    if (priv->storage_plugin)
> +      {
> +        g_object_unref (priv->storage_plugin);
> +        priv->storage_plugin = NULL;
> +      }

Non-merge-blocker: if you depend on telepathy-glib 0.11.7 (which I think
current MC master already needs), you can replace this block with:

    tp_clear_object (&priv->storage_plugin);

> +    const gchar *account, GValue *identifier)

One arg per line in definitions, please.

>  mcp_account_storage_get_identifier (const McpAccountStorage *storage,
...
> +  g_return_if_fail (!G_IS_VALUE (identifier));

This will be happy with identifier == NULL, which is also an error. Add:

    g_return_if_fail (identifier != NULL);

> + * Returns: a caller owned #GHashtable mapping of #gchar keys and #GValue
> + * values.

"#GHashTable" (note capitalization)

> +  if (iface->get_additional_info != NULL)
> +    rv = iface->get_additional_info (storage, account);
> +
> +  if (iface->get_additional_info == NULL || rv == NULL)
> +    rv = g_hash_table_new (g_str_hash, g_str_equal);

You can simplify the second "if" to "if (rv == NULL)", since you initialize rv
at the beginning. FYI we typically call the thing to be returned "ret", but
"rv" is fine too.

> + *   mcp_account_storage_iface_implement_get_dentifier (iface,

"identifier"

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



More information about the telepathy-bugs mailing list