GNOME Bugzilla – Bug 767690
[review] split GTK-editor part of nm-openconnect VPN properties plugin [th/vpn-editor-split-bgo767690]
Last modified: 2016-06-16 15:34:02 UTC
Same thing was done for nm-openvpn, nm-libreswan, nm-vpnc and nm-pptp. Please review
build/trivial: rename define NM_LIBRESWAN_OLD to NM_VPN_OLD Might want s/libreswan/openconnect/ on that. Still reading...
build: move "src/nm-openconnect-service.h" to "shared/nm-service-defines.h" You now have a file in shared/ which isn't actually shared. That seems... wrong.
Other than that, looks good. Thanks!
(In reply to David Woodhouse from comment #2) > build: move "src/nm-openconnect-service.h" to > "shared/nm-service-defines.h" > > You now have a file in shared/ which isn't actually shared. That seems... > wrong. "shared" first and foremost means: shared between the individual parts inside nm-openvpn. Previously, code in properties/ would include a header from src/. Secondly, the files inside "shared/nm-utils/" are somewhat special, in that they were copied as-is from https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/shared/nm-utils?id=89d32944af5a64c864cb3ad421d198206680b383 The other files inside "shared/" (except "shared/nm-utils/") are not copied from somewhere else. Ok, you will also find shared/nm-default.h in other plugins too and it serves the same purpose in those places, but it's different in that it doesn't have the notion of copied-as-is. That is indeed a subtle distinction. Fixed a few issues with the branch and merged to master: https://git.gnome.org/browse/network-manager-openconnect/commit/?id=2ead6c4bbb05e2e14869b815e860696dc775a185