Code review comment for lp://qastaging/~morphis/network-manager/add-snappy-support

1) I will have to say, I'm not a fan of merging the debian and snappy packaging into a single branch. There's simply too many debian-specific files included which are useless when it comes to building a snap. Add in the fact that the debian packaging branch for NM includes many stale patches ( ie. patches that aren't used or are commented out in the d/p/series ), and it seems like a mess to me.

Also, if we base the snap on the NM branch like this, we probably should revise the version to actually reflect this, at the moment it doesn't include the '-4ubuntu15.1.8' part in the version, yet it does include the code from that version.

I really prefer the bluez approach, where although the bzr namespace is the same as the debian package, the snap branch *only* includes snappy bits.

2) Why bother with --configure-snappy, the branch is solely for snappy? If we eventually try to upstream the patch(es), we'll probably need to change this is done ( ie. doubtful upstream would accept such a switch, we'd probably instead want to fix generically ), so let's either make the branch snappy only, or re-work the snappy patch into something we *think* upstream might accept ( note, let's do this after we switch to 1.2 ).

3) Why did you remove modemmanager and leave ofono in place? We know that modemmanager works, and we also know that ofono won't work till we re-write the settings plugin.

4) What's the reasoning behind the fix-code-to-build-with-werror.patch? It seems like this is changing a lot of code. Also this may not work with NM1.2, as they've gotten rid of some deprecated code ( eg. dbus-glib ), but not all ( eg. GSimpleAsyncResult ). Does the snap fail to build without this?

Note, I've also cleaned up a lot of ofono unused code in my 1.2 branch.

5) Where did the icon.svg come from? It's *huge* compared to the bluez icon.png ( which is a generic Ubuntu icon ).

6) What's the plan to fix "XXX: hardcode architecture" in wrapper binaries?

7) It looks like you're manually installing the binary wrapper scripts. Have you tested this? I couldn't get bluez to work this way, and ended up modifying yet another core snapcraft library to add the copies in when generating the wrapper scripts.

8) I don't see nmcli?

9) d/p/0002-Force-online-state-with-unmanaged-devices.patch: not re-patched for snappy.

10) 0003-Don-t-setup-Sleep-Monitor-if-not-booted-with-systemd.patch: not applicable for snappy.

11) d/p/0004-Use-symlinks-for-nmtui.patch: have you tested nmtui on snappy?

12) There are bunch of other patches that aren't really applicable to snappy ( eg. whoopsie support, upstart patches, ... ).