D15093: Add WireGuard capability.

Pino Toscano noreply at phabricator.kde.org
Mon Aug 27 06:42:03 BST 2018


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


  Misc notes:
  
  - please remove all the translations (i.e. `Name=[lang]` & `Comment[lang]` from desktop/json files, since KDE has an automatic system to handle them
  - please use KConfig <https://api.kde.org/frameworks/kconfig/html/classKConfig.html> to read & write ini-like files, instead of doing everything manually
  - the `PasswordField` class is already available as `libs/editor/widgets/passwordfield.h`, part of the `plasmanm_editor` library, so you do not need to copy it locally
  - `wireguard_global.h` seems not used, so just drop it
  - `nm-wireguard-service.h` has lots of `NM_OPENVPN_` keys that are not used
  - as @lbeltrame said, please use `QHostAddress` to parse IP addresses
  - as @lbeltrame said, hardcoding colors is a bad idea, and it will not play nice with user choice of different color schemes, or accessibility purposes
  - a better option to validate an input in a line edit is to use the embedded validator options, see `QLineEdit::setInputMask` and `QValidator`
  - the better option to edit a port number is `QSpinBox` restricted to 0-65536, instead of a line edit
  - there is a mixture of `virtual` & `Q_DECL_OVERRIDE` for virtual methods; plasma-nm uses `override` exclusively
  - please do not hardcode sizes and fonts in UI files
  - there are various texts in UI files with double spaces between words, please simply them to a single space
  - manually calling `KAcceleratorManager::manage(this);` is not needed, why were they added?

INLINE COMMENTS

> plasmanetworkmanagement_wireguardui.desktop:7-8
> +X-NetworkManager-Services=org.freedesktop.NetworkManager.wireguard
> +X-KDE-PluginInfo-Author=Lukáš Tinkl
> +X-KDE-PluginInfo-Email=lukas at kde.org
> +X-KDE-PluginInfo-Name=plasmanetworkmanagement_wireguardui

Surely this was not done by Lukáš... please fill in your name/email.

> wireguardadvanced.ui:39
> +       <property name="text">
> +        <string>Intreface</string>
> +       </property>

typo, "Interface:"

REPOSITORY
  R116 Plasma Network Management Applet

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

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


More information about the Plasma-devel mailing list