[Bug 33252] Refactoring of TplEntity

[bugzilla-daemon at freedesktop.org]

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

Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
         Depends on|                            |33212

--- Comment #3 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2011-01-19 10:07:52 PST ---
(In reply to comment #2)
> +++ b/telepathy-logger/channel-text.c
> +    _tpl_event_set_id (log, _tpl_channel_text_get_chatroom_id ( tpl_text));
> 
> Remove whitespace after parenthesis.

Done.

> 
> --- a/telepathy-logger/entity.c
> +++ b/telepathy-logger/entity.c
> 
> +      case PROP_TYPE:
> +        priv->type = g_value_get_int (value);
> 
> Shouldn't it be g_value_get_uint (since it's an enum) ?

No because enums are int in C.

> 
> -      G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
> +      G_PARAM_READWRITE | G_PARAM_CONSTRUCT | G_PARAM_STATIC_STRINGS);
> 
> Use G_PARAM_CONSTRUCT_ONLY.

That was my intent, Done.

> 
> -        _tpl_entity_set_alias (self, g_value_get_string (value));
> +        g_free (priv->alias);
> +        priv->alias = g_value_dup_string (value);
> 
> Since they are construct-only, instead g_free(), do
> g_assert (priv->alias == NULL); /* construct-only */
> Same for the other construct-only properties.

Done.

> 
> 
> +  param_spec = g_param_spec_int ("type",
> +      "Type",
> +      "The entity's type",
> +      TPL_ENTITY_UNKNOWN,
> +      TPL_ENTITY_SELF,
> +      TPL_ENTITY_UNKNOWN,
> +      G_PARAM_READWRITE | G_PARAM_CONSTRUCT | G_PARAM_STATIC_STRINGS);
> 
> Not sure if you should set the maximum to TPL_ENTITY_SELF, since if at some
> point we add a new type, we'll probably forget to update this. Also I've
> usually seen G_MAXINT there.

Yeah I know, I'm being laze because the param_spec_enum() exist bug requires a
bit a work to get the stuff generated. I'll ask others about that.

> 
> Looks quite good otherwise!

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