D20930: Update WireGuard to match NetworkManager 1.16 interface

Jan Grulich noreply at phabricator.kde.org
Thu May 2 14:19:53 BST 2019


jgrulich added a subscriber: ngraham.
jgrulich added a comment.


  Can we maybe have some assistence from anyone from VDG? @ngraham maybe?

INLINE COMMENTS

> CMakeLists.txt:11
>  
> +set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g")
> +

Please remove.

> kcm.cpp:264
>      NetworkManager::ConnectionSettings::ConnectionType type = static_cast<NetworkManager::ConnectionSettings::ConnectionType>(connectionType);
> -
> +qCWarning(PLASMA_NM) << "onRequestCreateConnection type: " << type;
>      if (type == NetworkManager::ConnectionSettings::Vpn && vpnType == "imported") {

Please remove.

> connectionicon.cpp:403
>                  }
> +            } else if (type == 29) {      // TODO change to WireGuard enum value once it is added
> +                // WireGuard is a VPN but is not implemented

Do you want me to add this to NMQT?

> connectioneditorbase.cpp:151
>      }
> -
> +#if 1
> +qCWarning(PLASMA_NM) << "ConnectionEditorBase: Before toMap";

Please remove

> connectioneditorbase.cpp:198
>  
> +    qCWarning(PLASMA_NM) << "ConnectionEditorBase:" << m_connection->id();
> +

Please remove.

> connectioneditorbase.cpp:264
>          if (!vpnSetting) {
> -            qCWarning(PLASMA_NM) << "Missing VPN setting!";
> +            qCWarning(PLASMA_NM) << "Missing VPN setting 2!";
>          } else {

Why the "2" at the end?

> connectioneditorbase.cpp:458
>          for (const QString &key : secrets.keys()) {
> +            qCWarning(PLASMA_NM) << "replyFinished key: " << key << "  settingName: " << settingName;
>              if (key == settingName) {

Please remove.

> connectioneditorbase.cpp:465
>                          const QString type = widget->type();
> +qCWarning(PLASMA_NM) << "type: " << type << "   settingName: " << settingName;
>                          if (type == settingName ||

Please remove.

> wireguardinterfacewidget.cpp:114
> +WireGuardInterfaceWidget::WireGuardInterfaceWidget(const NetworkManager::Setting::Ptr &setting, QWidget* parent, Qt::WindowFlags f):
> +    SettingWidget(setting, parent, f),
> +    d(new Private)

Coding style.

Should be:

  WireGuardInterfaceWidget::WireGuardInterfaceWidget(const NetworkManager::Setting::Ptr &setting, QWidget* parent, Qt::WindowFlags f)
      : SettingWidget(setting, parent, f)
      , d(new Private)

> wireguardinterfacewidget.cpp:182
> +
> +    if (0 != wireGuardSetting->listenPort())
> +        d->ui.listenPortLineEdit->setText(QString::number(wireGuardSetting->listenPort()));

I would preffer:

  if (wireGuardSetting->listenPort() != 0)

It's more readable. Same for code below.

> wireguardinterfacewidget.cpp:250
> +{
> +
> +    NetworkManager::WireGuardSetting wgSetting;

Remove empty line

> wireguardinterfacewidget.cpp:424
> +
> +        else if (keyValue[0] == PNM_WG_CONF_TAG_PEER) {
> +            // Check to make sure the previous PEER section has

Coding style. The "else if" should be on the same line as the "}" bracket.

> wireguardinterfacewidget.cpp:482
> +            // Listen Port
> +            else if (key == PNM_WG_CONF_TAG_LISTEN_PORT) {
> +                uint val = keyValue[1].toUInt();

Same here and below.

> wireguardinterfacewidget.cpp:568
> +        else if (currentState == PEER_SECTION)
> +        {
> +            // Public Key

The "{" bracket should be on the same line as the "else if". Same below.

> wireguardinterfacewidget.cpp:633
> +        peers.append(*currentPeer);
> +#endif
> +#if 1

Please remove

> wireguardpeerswidget.cpp:82
> +
> +WireGuardPeersWidget::WireGuardPeersWidget(const NMVariantMapList& peerData, QWidget* parent, Qt::WindowFlags f):
> +    QDialog(parent, f),

Coding style.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D20930

To: andersonbruce, jgrulich
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190502/2d006be0/attachment-0001.html>


More information about the Plasma-devel mailing list