[Bug 70095] Audit API in util.h and deprecate/remove what got into glib

[bugzilla-daemon at freedesktop.org]

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

--- Comment #3 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
I'm less keen on this than I am on cleaning up more closely Telepathy-related
things. Having extra code in util.c doesn't cost us much, it doesn't "paint us
into a corner" in the same way that actual Telepathy APIs might, and it doesn't
make subtle bugs more likely in the same way that not using TpClientFactory
can.

(In reply to comment #0)
> * tp_g_ptr_array_contains: Not included yet:
> https://bugzilla.gnome.org/show_bug.cgi?id=698064

Certainly keep

> * tp_g_ptr_array_extend: Not included, I don't think we have a bug for it.
> But IIRC we use it for dbus-glib stuff so probably not needed with GVariant ?

Keep if anything uses it

> * tp_g_value_*: Not useful in a GVariant world, so probably not worth
> upstreaming.

Certainly keep until we eradicate dbus-glib, and tbh it costs ~nothing

> * tp_g_hash_table_update: not sure how useful it is when using GVariant...

It's basically Python's dict.update(). Please keep it, for use with
general-purpose mutable data structures.

> * tp_str_empty: I can't believe they are seriously rejecting that...
> https://bugzilla.gnome.org/show_bug.cgi?id=399880

Keep it, it doesn't add any shared-object code at all.

> * tp_strdiff: same: https://bugzilla.gnome.org/show_bug.cgi?id=685878

With hindsight, it should have been tp_strequals0 or something (opposite
sense), but yes I think we still want this one.

> * tp_escape_as_identifier: do we want to upstream?

I'm not sure we do. It's quite domain-specific ("mangle names into a really
really limited character set") and I've been careful to avoid having the exact
encoding be API.

> * tp_strv_contains: not there yet:
> https://bugzilla.gnome.org/show_bug.cgi?id=685880

Keep.

> * tp_g_signal_connect_object: I think we should deprecate it and remove from
> next.

Does it cost us anything?

If you want to get rid of it, step 1 is to implement it in terms of its
replacement or analyze why that isn't safe, IMO.

> * tp_value_array_build, tp_value_array_unpack, tp_value_array_free: Not
> useful with GVariant.

We can get rid of them when we move to GDBus.

> * TpWeakRef: GLib has GWeakRef now. It's not exactly the same since it does
> not have extra gpointer user_data and it's missing g_weak_ref_new(). It
> should be easy to get g_weak_ref_new() upstream and maybe add
> tp_g_weak_ref_new() in the meantime. Should we deprecate TpWeakRef and
> remove from next?

Its semantics aren't quite the same and having the user-data is pretty
convenient. It doesn't cost us much.

Many things that are using TpWeakRef should ideally be using GTask, if
anything.

A large part of the motivation for TpWeakRef working the way it does was "be
able to implement our deprecated pre-GAsyncResult APIs in terms of a
GAsyncResult". Hopefully most of that has gone away.

> * tp_clear_pointer: Can be deprecated/removed if we don't mind the extra
> threadsafeness of g_clear_pointer.

What does it cost us?

We could turn it into a #define for g_clear_pointer.

> * tp_clear_object: Can be deprecated/removed.

What does it cost us? But, yes.

> * tp_simple_async_report_success_in_idle: We should deprecate/remove in
> favor of GTask API.

One day, but that day is not today IMO. Let's not block 1.0 on it.

> * tp_user_action_time_from_x11, tp_user_action_time_should_present: Makes no
> sense to upstream, right?

No, this is Telepathy-API-specific.

> * tp_utf8_make_valid: Still not in glib:
> https://bugzilla.gnome.org/show_bug.cgi?id=610969

Keep it.

> We also have util-internal that can be cleaned:
> 
> * _tp_implement_finish_*: We should use GTask instead which is easier to
> implement _finish() functions.

Please don't spend a lot of time on it, or block 1.0 on it.

I agree that new code should be GTask.

> * _tp_object_list_copy: I think g_list_copy_deep(list, g_object_ref) should
> be good enough now.

Only if it's quick to do.

> * _tp_object_list_free: g_list_free_full(list, g_object_unref) should be
> enough.

Only if it's quick to do.

> * _tp_g_list_copy_deep: we have g_list_copy_deep since 2.34

Only if it's quick to do.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.