Antti, I this is a big undertaking, now I understand how pete-woods must've felt when I asked him to review indicator datetime-cpp.

Here are some notes I took while reading this morning & this afternoon, I got through some of the build stuff and through the gio utils and gmenu abstraction. This is a long list but most of the bullet points are minor.

The big takeaway I got from what I read today is that I really like these pieces; you should consider extracting them out so that they could be used by other indicators too. I've gone through some of the same questions of "how much of the toolkit pieces should I abstract out?" but menumodel-cpp goes much much further than I did...

* in network/dbus-cpp/services/ofono.h, license is LGPLv3 unlike all the rest of the code

* in COPYING.LGPL, this file should be removed

* in README: still says the system is built with autogen

* in README.widgets is nice. I'm going to steal this a a template. :)

* in debian/control, building still requires valac

* in README.widgets, why do the widgets have an x-canonical-widget-type of 'unity.widgets.systemsettings.${profile}.widgettype' instead of 'com.canonical.indicator.widgettype'?

* In debian/control, indicator-network still conflicts with chewie. Is this line still relevant?

* in CMakeLists.txt, a lot of the pkg_check_modules() + include_directories() pairs are only needed for tests. If they were moved into tests/CMakeLists.txt it would avoid polluting the include path when building the rest of the code

* in CMakeLists.txt, why the duplicate test for gio-2.0>=${GLIB_REQUIRED_VERSION} ?

* in gio-helpers/*, inconsistent use of assert: assert() in variant.h, g_assert() in util.h

* in gio-helpers/* and menumodel-cpp/*, lots header bloat: most of this could be moved into .cpp files

* in gio-helpers/util.h, why #define _(String) instead of including gi18n.h?

* in gio-helpers/util.h, why use std::cerr instead of g_warning()?

* in gio-helpers/util.h, SessionBus::address() should be a const method

* in gio-helpers/util.h, SessionBus::address() the second 'error = NULL' isn't needed

* in gio-helpers/util.h, GMainLoopSync appears to be unused code & can be removed

* in gio-helpers/util.h GMainLoopDispatch::dispatch_cb(), funcPtr should be auto

* in gio-helpers/util.h, SessionBus::SessionBus() should use g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)

* in gio-helpers/variant.h, Variant::as() should be a const method

* in gio-helpers/variant.h, what's the decision process for deciding which methods got the 'inline' keyword?

* in gio-helpers/variant.h, debug messages have leaked into cout. I =hate= this; suggest g_debug() instead

* in gio-helpers/variant.h Codec::vector<Variant>::encode_argument(), "auto value" would be slightly better as "auto& value"

* in gio-helpers/variant.h, #include warts: (1) uses string w/o including it (2) uses std::move() w/o including <utility)

* in menumodel-cpp, lots of use of std::lock_guards, not disagreeing with this but I'm curious about the context -- does i-network have thread contention issues with menus?

* in menumodel-cpp/menu.h, #include warts: (1) includes map w/o using it, (2) includes vector w/o using it

* in menumodel-cpp/action.h, Action::name() should be a const method

* in menumodel-cpp/action.h Action::~Action(), I'm not sure why a GMainLoopDispatch is needed here instead of calling
 g_signal_handler_disconnect() directly. I don't think disconnect() is unsafe?

* in menumodel-cpp/action.h Action::setState(), agree about making the state a core::Property

* in menumodel-cpp/action-group.h, #include warts: (1) uses std::string w/o including it, (2) uses cerr w/o including iostream (better to remove the cerr use -- throw or use a g_warning()?) (3) headers are #included out of order

* in menumodel-cpp/action-group.h, ActionGroup::contains() should be a const method

* in menumodel-cpp/action-group-exporter.h, m_exportId should be a guint

* in menumodel-cpp/action-group-exporter.h ActionGroupExporter::ActionGroupExporter(), should use g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)

* in menumodel-cpp/action-group-merger.h, #include warts: (1) includes set w/o using it (2) uses string w/o including it (3) headers are #included out of order

* in menumodel-cpp/action-group-merger.h, ActionGroupMerger::add() and ::remove(): "auto action" would be slightly better as "auto& action"

* in menumodel-cpp/action-group-merger.h, ActionGroupMerger::remove(), need to disconnect from that group's actionAdded() and actionRemoved() signals

* in menumodel-cpp/action-group-merger.h, ActionGroupMerger::~ActionGroupMerger(), need to disconnect from all groups' actionAdded() and actionRemoved() signals? Not sure about this one.

* in menumodel-cpp/menu-merger.h, MenuMerger never disconnects from the "items-changed" signal

* in menumodel-cpp/menu-model.h, #include warts: (1) reference to GMenuModel w/o including gio, (2) use of shared_ptr w/o including memory