Created attachment 335930 [details] [review]
telepathyClient: Implement Tp.BaseClient directly

The telepathy integration was written at a time where gjs didn't
allow to inherit from GObject classes, which is why we needed a
C helper class. This hasn't been the case for a while now, so cut
out the middle man and implement Tp.BaseClient directly.

Created attachment 335931 [details] [review]
shell: Remove TpClient

With the telepathy component now being implemented purely in JS, we
can drop the C helper class and therefore the linker dependency on
telepathy-glib.

Review of attachment 335930 [details] [review]:

lgtm

Review of attachment 335931 [details] [review]:

good riddance

It fails to build with those patches:
main.c:23:34: fatal error: telepathy-glib/debug.h: No such file or directory
 #include <telepathy-glib/debug.h>

Yes, there's some logging special-casing in main.c - I haven't quite made up my mind what to replace it with, which is why those patches haven't been pushed yet.

Created attachment 347577 [details] [review]
shell: Remove TpClient

With the telepathy component now being implemented purely in JS, we
can drop the C helper class.

Comment on attachment 347577 [details] [review]
shell: Remove TpClient

(Restoring patch status, as I just moved the configure.ac bits to the next patch)

Created attachment 347578 [details] [review]
main: Remove telepathy-specific log handling

The built-in telepathy integration is far less prominent than it
used to be, and doesn't really justify a link time dependency to
send debug message over D-Bus.

Review of attachment 347578 [details] [review]:

sure

::: src/main.c
@@ +43,3 @@
 #define DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER 4
 
+#define DBUS_NAME_FLAG_REPLACE_EXISTING 0x2

seems unrelated?

(In reply to Rui Matos from comment #10)
> Review of attachment 347578 [details] [review] [review]:
> 
> sure
> 
> ::: src/main.c
> @@ +43,3 @@
>  #define DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER 4
>  
> +#define DBUS_NAME_FLAG_REPLACE_EXISTING 0x2
> 
> seems unrelated?

actually, I guess we should use G_BUS_NAME_OWNER_FLAGS_REPLACE which we already do for other DBus names anyway

Created attachment 347655 [details] [review]
main: Use GBusNameOwnerFlags to specify flags

We are still using DBUS_NAME_FLAG_REPLACE_EXISTING from dbus-glib
in some places (whose headers are drawn in via telepathy-glib).
As both GIO and dbus-glib use the values that will end up being
sent on the bus, the define and enum value are interchangeable,
but it's clearly better to consistently use the library that is
explicitly included.


(In reply to Rui Matos from comment #11)
> > +#define DBUS_NAME_FLAG_REPLACE_EXISTING 0x2
> > 
> > seems unrelated?

It isn't, as the symbol comes from dbus-glib which is only included via telepathy-glib. But ...


> actually, I guess we should use G_BUS_NAME_OWNER_FLAGS_REPLACE which we
> already do for other DBus names anyway

... obviously that's far better.

Review of attachment 347655 [details] [review]:

++

Attachment 335930 [details] pushed as 7c96b39 - telepathyClient: Implement Tp.BaseClient directly
Attachment 347577 [details] pushed as 3c828c8 - shell: Remove TpClient
Attachment 347578 [details] pushed as 2d814bf - main: Remove telepathy-specific log handling
Attachment 347655 [details] pushed as 8f8e512 - main: Use GBusNameOwnerFlags to specify flags