D15093: Add WireGuard capability.

[Jan Grulich]

jgrulich added a comment.


  I think you can completely remove WireguardAuth dialog if there is no use for it. I also spotted few trailing spaces in the patch, please remove them.

INLINE COMMENTS

> wireguard.cpp:189
> +            } else { // Error condition
> +                return result;
> +            }

You multiple times return just result, you should also set error message with the reason so users know why the import failed.

> wireguardadvancedwidget.cpp:104
> +
> +    setOrClear(data, QLatin1String(NM_WG_KEY_LISTEN_PORT), m_ui->listenPortLineEdit->displayText());
> +    setOrClear(data, QLatin1String(NM_WG_KEY_MTU), m_ui->mTULineEdit->displayText());

I would prefer having just an empty map with data where you just set everything the user configured in UI, removing options from existing data map might work, but if someone configure a connection somewhere else with options we don't support, they will stay there as you will not remove them. Also change the setOrClear() function to something like setProperty(const NMStringMap &data, const QString &key, const QString &value).

> wireguardwidget.cpp:31
> +
> +#include <KProcess>
> +#include <KUrlRequester>

Not needed.

> wireguardwidget.cpp:129
> +{
> +    // Currently WireGuard does not have any secrets
> +}

Add Q_UNUSED(setting);

> wireguardwidget.h:33
> +
> +class QUrl;
> +class QLineEdit;

QUrl and QLineEdit not needed.

> wireguardwidget.h:41
> +    explicit WireGuardSettingWidget(const NetworkManager::VpnSetting::Ptr &setting,
> +                                    const struct WireGuardRegexStrings &strings,
> +                                    QWidget *parent = 0);

This paramater can be removed if you switch to using our IPv4 and IPv6 validators, also you can completely remove wireguardstrings structure you defined. Even if anything like this would be needed, you basically need it in one class, why not defining it there? If our validators are not enought, then they should be improved so every class using them can benefit from that.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: andersonbruce, #plasma, jgrulich, pino
Cc: acrouthamel, K900, anthonyfieroni, pino, lbeltrame, ngraham, plasma-devel, 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/20180905/4a27753e/attachment-0001.html>