D15093: Add WireGuard capability.

Luca Beltrame noreply at phabricator.kde.org
Mon Aug 27 05:36:14 BST 2018


lbeltrame added a comment.


  Some smallish nitpicks I noticed there. Also, does this mean that NM has WG support now?
  
  I'll also let @jgrulich comment on this, but I'm not too fond of the manual parsing of the INI-like file used by WG. While I can't check at the moment, there should be KF5 or Qt classes to handle tihs.

INLINE COMMENTS

> passwordfield.h:2
> +/*
> +    Copyright 2015 Jan Grulich <jgrulich at redhat.com>
> +

Please add your name to copyright if you wrote most of the code (likewise elsewhere in the code, and in the .desktop file.

> wireguardutils.cpp:42
> +// and/or a subnet (separated by the rest by a slash; 0 - 32)
> +bool WireGuardUtils::is_ip4(QString addr, bool allow_subnet, bool allow_port)
> +{

You may want to use QHostAddress instead of doing your own parsing of IPs: https://stackoverflow.com/questions/46853422/how-to-tell-if-qhostaddress-is-ipv4-or-ipv6-in-qt5

> wireguardwidget.cpp:296
> +    {
> +        d->ui.endpointLineEdit->setStyleSheet("* { background-color: rgb(255,128, 128) }");
> +    }

Why this? It will break proper coloring with people that use custom themes. IIRC you can have the widget handle it (but I can't check at the moment).

REPOSITORY
  R116 Plasma Network Management Applet

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

To: andersonbruce, #plasma, jgrulich
Cc: 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/20180827/c1082a85/attachment.html>


More information about the Plasma-devel mailing list