D15093: Add WireGuard capability.

[Pino Toscano]

pino requested changes to this revision.
pino added a comment.
This revision now requires changes to proceed.


  note there are still few "not done" comments around (eg using QSpinBox for fwMark)

INLINE COMMENTS

> CMakeLists.txt:28
>      Widgets
> +    Test
>  )

already added by D15520: Upgrade SimpleIpV4AddressValidator and SimpleIpV6AddressValidator <https://phabricator.kde.org/D15520>

> wireguard.cpp:139
> +
> +        WireGuardKeyValidator keyValidator(this);
> +        int keyPos = 0;

no need to specify `this` as parent, since it is on stack, and thus it will be deleted automatically

> wireguard.cpp:139-140
> +
> +        WireGuardKeyValidator keyValidator(this);
> +        int keyPos = 0;
> +

indent

> wireguard.cpp:177
> +    if (!value.isEmpty()) {
> +        int pos = 0;
> +        SimpleIpListValidator validator(this,

since earlier there is a `keyPos` variable used for basically the same purpose (i.e. passing it to the `validate()` of validators), just move `pos` earlier in the function, and use one everywhere

> wireguardadvancedwidget.cpp:26
> +
> +#include <QStandardPaths>
> +#include <QUrl>

unused

> wireguardadvancedwidget.cpp:31
> +#include <KLocalizedString>
> +#include <KProcess>
> +#include <KAcceleratorManager>

unused

> wireguardadvancedwidget.cpp:120
> +    NMStringMap data;
> +    long intVal;
> +    QString stringVal;

a bit confusing to call `intVal` a variable which is (not correctly) a long; OTOH, it can be removed altogether (see below)

> wireguardadvancedwidget.cpp:121
> +    long intVal;
> +    QString stringVal;
> +

unused

> wireguardadvancedwidget.cpp:123-127
> +    intVal = m_ui->listenPortSpinBox->value();
> +    setOrClear(data, QLatin1String(NM_WG_KEY_LISTEN_PORT), intVal);
> +
> +    intVal = m_ui->mtuSpinBox->value();
> +    setOrClear(data, QLatin1String(NM_WG_KEY_MTU), intVal);

just inline the calls to `value()`, just like done below

> wireguardadvancedwidget.h:27
> +#include <QDialog>
> +#include <QProcess>
> +

unused

> wireguardkeyvalidator.h:26
> +
> +class Q_DECL_EXPORT WireGuardKeyValidator : public QValidator
> +{

no need for `Q_DECL_EXPORT`, as it is in a plugin, and thus it does not need to be exported as public symbol

> pino wrote in wireguardwidget.cpp:182
> no need for the (...) for the whole condition:
> 
>   return foo || bar;
> 
> is easier than
> 
>   return (foo || bar);

this is not "done", btw

REPOSITORY
  R116 Plasma Network Management Applet

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

To: andersonbruce, #plasma, jgrulich, pino
Cc: acrouthamel, K900, 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/20180918/0b8c050b/attachment-0001.html>