[Bug 33213] Rename _get_events() to get_entities()

[bugzilla-daemon at freedesktop.org]

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

--- Comment #2 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2011-01-19 12:43:54 PST ---
(In reply to comment #1)
> +/* This macro is used to check if a list has been taken by an _finish()
> + * function call. It detects the the marker set by _take_list() method. Those
> + * are use to avoid copying the full list on every calls. */
> 
> an _finish -> a _finish
> the the -> the
> are use -> are used
> every calls -> every call

Done.

> 
> -      _search_in_identifier_async_result_free);
> +      (GDestroyNotify) tpl_log_manager_search_free);
> 
> -      _search_new_async_result_free);
> +      (GDestroyNotify) tpl_log_manager_search_free);
> 
> I don't think you need the cast here.

The arg1 type is not a void*, thus GCC complains, that's why it's required.

> 
> Please split this commit in two, one for the free funcs and another for the
> _take_list.

Ok, I've merged them because I originally wrote them in reverse order, which I
found confusing.

Done.

> 
> For the second commit:
> 
> The _get_events() method was miss-leading. This function would return
> partially filled TplLogSearchHit that would in fact represent entities.
> This patch rename the function an return GList of TplEntity.
> 
> miss-leading -> misleading
> rename -> renames
> an return -> and returns a
> 
> 
> -GList * _tpl_log_manager_get_events (TplLogManager *manager,
> +GList * _tpl_log_manager_get_entities (TplLogManager *manager,
> 
> OK, that's fine as that's what that function is doing anyway. We may want to
> write a tpl_log_manager_get_events() function that actually returns a list of
> TplEvents (that would be needed for my new Empathy viewer proposal for
> example).

Adding functions is not an issue for ABI. Note that getting all events for an
account is a bad idea. If they want to do like the N900 we will implement a two
way iterator or something similar. Do you have a draft for this proposal ?

> 
> 
> -gint _tpl_log_manager_search_hit_compare (TplLogSearchHit *a,
> -    TplLogSearchHit *b);
> +/*gint _tpl_log_manager_search_hit_compare (TplLogSearchHit *a,
> +    TplLogSearchHit *b);*/
> 
> Just remove it.

Oops, left over.

> 
> 
> + * _tpl_entity_compare:
> + * @a: a #TplEntity
> + * @b: a #TplEntity
> +_tpl_entity_compare (TplEntity *e1,
> +    TplEntity *e2)
> 
> Either rename e1/e2 or @a/@b. Also make it consistent with the header if you
> decide to make it a/b.

Done, used a/b.

> 
> -  g_return_val_if_fail (a != NULL && a->id != NULL, 1);
> -  g_return_val_if_fail (b != NULL && b->id != NULL, 1);
> 
> Why remove the != NULL check? If we keep them they should become TPL_IS_ENTITY
> though.

Added with TPL_IS_ENTITY(). Note that I did something tricky to guaranty finite
order:
  g_return_val_if_fail (TPL_IS_ENTITY (a), TPL_IS_ENTITY (b) ? -1 : 0);
  g_return_val_if_fail (TPL_IS_ENTITY (b), 1);

> 
> 
> - * Returns: a list of pointer to TplLogSearchHit, having id and
> - * type fields filled. The result needs to be freed after use, see
> - * _tpl_log_manager_search_hit_free()
> + * Returns: a list of pointer to #TplEntity, The result needs to be freed
> after
> + * use.
> 
> Do the TplEntities need to be unreffed? Please mention that there too.

Yes, fixed.

> 
> + * tpl_log_manager_get_entities_finish:
> 
> + * @chats: a pointer to a #GList used to return the list of #TplEntity
>      GList **events,
> 
> events should be renamed to entities I guess. And @chats too.
> 
> 
> -gboolean tpl_log_manager_get_events_finish (TplLogManager *self,
> +gboolean tpl_log_manager_get_entities_finish (TplLogManager *self,
>      GAsyncResult *result,
>      GList **events,
> 
> Likewise.

Done.

> 
> 
> +      /* FIXME Facking alias with identifier */
> 
> Faking I think.

Done.

Branch updated too.

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