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