After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 767690 - [review] split GTK-editor part of nm-openconnect VPN properties plugin [th/vpn-editor-split-bgo767690]
[review] split GTK-editor part of nm-openconnect VPN properties plugin [th/vp...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: VPN: openconnect
git master
Other Linux
: Normal normal
: ---
Assigned To: David Woodhouse
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-06-15 13:22 UTC by Thomas Haller
Modified: 2016-06-16 15:34 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Thomas Haller 2016-06-15 13:22:00 UTC
Same thing was done for nm-openvpn, nm-libreswan, nm-vpnc and nm-pptp.

Please review
Comment 1 David Woodhouse 2016-06-15 20:16:45 UTC
    build/trivial: rename define NM_LIBRESWAN_OLD to NM_VPN_OLD

Might want s/libreswan/openconnect/ on that. Still reading...
Comment 2 David Woodhouse 2016-06-15 20:18:07 UTC
    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.
Comment 3 David Woodhouse 2016-06-15 20:21:12 UTC
Other than that, looks good. Thanks!
Comment 4 Thomas Haller 2016-06-16 15:34:02 UTC
(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