Bug 96707

I wonder how long until telepathy-glib breaks too then.

Created attachment 124774 [details] [review]
Port to using the new nm 1 API

Technically libnm is using gio/gio.h and the rest of telepathy is still using dbus-glib so that will eventually break too.

Testing was to disable my normally installed mission-control-5 by moving it out of the way, and running the new mission-control-5 from source. simple gabble resources and idle connected, but my google resources didn't load.

How does it work for you?

(In reply to diane from comment #2)
> Created attachment 124774 [details] [review] [review]
> Port to using the new nm 1 API
> 
> How does it work for you?

I've applied your patch and successfully compiled telepathy-mission-control with NetworkManager support:

# ldd /usr/x86_64-pc-linux-gnu/libexec/mission-control-5
libnm.so.0 => /usr/x86_64-pc-linux-gnu/lib/libnm.so.0 (0x00007f1bcbc51000)

My already existing Jabber/XMPP connection works fine so far, I'm however not sure how to test some NetworkManager<->telepathy-mission-control related features in particular.

> 
> My already existing Jabber/XMPP connection works fine so far, I'm however
> not sure how to test some NetworkManager<->telepathy-mission-control related
> features in particular.

mission-control uses NetworkManager to see if you have a network connection. you should see your contact list go off-line when you disconnect from all networks, you should see it reconnect when slightly after re-establishing a network connection, and you should not see your client spinning trying to connect to your jabber server while you're disconnected.

Comment on attachment 124774 [details] [review]
Port to using the new nm 1 API

Review of attachment 124774 [details] [review]:
-----------------------------------------------------------------

Looks good to me. It looks like it should work fine.

In addition to your changes, I would also now remove NM_CHECK_VERSION check on line 205 and leave the "|| state == NM_STATE_DISCONNECTING" compile unconditionally (as we no longer support older versions), and also remove the NM_CHECK_VERSION definition on line 185-187 (this is to support even older versions that don't have the NM_CHECK_VERSION macro in the headers)

::: src/connectivity-monitor.c
@@ +459,4 @@
>  mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor)
>  {
>    McdConnectivityMonitorPrivate *priv;
> +  GError *error;

GError pointers must be initialized to NULL before use.

Created attachment 124791 [details] [review]
port to using new NM API

Fixed the GError initialization.

And removed the macro definition and use you mentioned in the review.

I first thought about putting removing the macros into a separate commit, but decided to merge them as it was all part of the logical change of moving to the new API.

Looks good. Please commit. I suggest committing it in the 5.16 branch; since master contains incompatible changes, I think we should make a new 5.16.x release at some point for this before jumping to the incompatible version.

Comment on attachment 124791 [details] [review]
port to using new NM API

Perhaps it'd be better if I didn't commit things just before going to sleep, as I apparently need to proofread my commit messages more carefully.

but pushed to telepathy-mission-control-5.16 as commit 91499340222a563bacce7994f32275c38ff55ff0