GNOME Bugzilla – Bug 734746
make NMRemoteConnection an NMObject [danw/remoteconnection-object]
Last modified: 2014-08-16 14:24:21 UTC
from the commit message: The fact that NMRemoteConnection has to be an NMConnection and therefore can't be an NMObject means that it needs to reimplement bits of NMObject functionality (and likewise NMObject needs some special magic to deal with it). Likewise, we will need a daemon-side equivalent of NMObject as part of the gdbus port, and we would want NMSettingsConnection to be able to inherit from this as well. My original plan had been to make NMObject an interface, but it ended up making more sense to make NMConnection an interface instead. The first commit ("all: fix up multiple-include-guard defines") has basically no direct connection to any of this, it's just that the following commits changed bits of libnm-core API that were being used by nm-test-utils.h, meaning that the test programs in libnm-util would no longer compile, because nm-test-utils.h didn't recognize the difference between libnm-core's nm-connection.h and libnm-util's nm-connection.h. So this makes it so that it can export nmtst_create_minimal_connection() only when using libnm-core, not when using libnm-util. I'm not totally sure about exposing "NMSimpleConnection"... it's possible that it would be better to just keep that as an implementation detail, and leave nm_connection_new(), etc, as they were?
> libnm-core: move some fake NMConnection methods over to NMSetting Do you think nm_setting_new_by_name() is still useful? The applet/editor don't use it in normal code (only in one place in the GConf migration code), and the only user in NM itself is 'keyfile' in one place. gnome-shell and the control center don't use it at all. Since it's so small, maybe we should just kill it entirely and move the logic to the sole user in keyfile? That said, it does abstract the details of name<->object-type mapping that's really a GObject implementation detail, so maybe it has use still. > libnm: get rid of redundant NMRemoteConnection properties Can we actually get rid of NM_OBJECT_DBUS_CONNECTION entirely, at least as public API? If NMClient is creating the D-Bus connection internally and then passing it down to all the other objects internally, and presumably (soon?) passing it to NMRemoteSettings and thus to NMRemoteCOnnection too, then I can't see where a client would need to care about it. Then we could switch it over to GDBusConnection too perhaps, or maybe you have a further plan for that. > libnm-core, libnm, core: make NMConnection an interface I don't have a problem with NMSimpleConnection, but it would make the patch a lot smaller if we left it as nm_connection_new(). However, do you think that would be confusing to API users? I guess the downside is that the function names don't follow the convention (where they have _ format of the object name and G_TYPE), so I guess I'm slightly on the side of NMSimpleConnection for clarity's sake. We can always add nm_connection_new() back later if we want too. Rest looks good.
(In reply to comment #1) > > libnm-core: move some fake NMConnection methods over to NMSetting > > Do you think nm_setting_new_by_name() is still useful? From your analysis it sounds like "no". > > libnm: get rid of redundant NMRemoteConnection properties > > Can we actually get rid of NM_OBJECT_DBUS_CONNECTION entirely, at least as > public API? We at least need a private property, to pass the connection down from the creator to the created. In this case, the property is construct-only and non-readable though, so it's practically like it's not there... (Also, properties can't *really* be private; there's no way to get gtk-doc to not document them, or to get bindings to not expose them, etc.) > Then we could switch it over > to GDBusConnection too perhaps, or maybe you have a further plan for that. Yeah, I was just going to change the type of the property. > I don't have a problem with NMSimpleConnection, but it would make the patch a > lot smaller if we left it as nm_connection_new(). Yeah, I had all the same thoughts you did. > so I guess I'm slightly on the side of NMSimpleConnection for > clarity's sake. We can always add nm_connection_new() back later if we want > too. Although "slightly on the side of keeping nm_connection_new() for simplicity's sake; we can always rename it later if we want too" makes just as much sense...
Yeah, true, so your call on Simple vs. nm_connection_new(). I'm OK either way.
dropped nm_connection_create_setting() and pushed (keeping explicit NMSimpleConnection) 9a9c2f4f844ed134d2ecb59b8bcda202b57d7c47