D17210: Added proxy and user settings

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> proxysetting.cpp:25
> +#include <QDebug>
> +
> +NetworkManager::ProxySettingPrivate::ProxySettingPrivate()

It looks that the proxy setting has been introduced in NetworkManager 1.6. This means that for all property defines, you have to add ifdef the same way you did for ip tunnel setting.

> proxysetting.h:35
> +/**
> + * Represents generic setting
> + */

Represents proxy setting

> proxysetting.h:52
> +
> +    void setMethod(quint32 method);
> +    quint32 method() const;

This can be turned into an enum.

Possible values are:
NM_SETTING_PROXY_METHOD_NONE = 0,
NM_SETTING_PROXY_METHOD_AUTO = 1,

So I would do:
enum Mode { None = NM_SETTING_PROXY_METHOD_NONE, auto = NM_SETTING_PROXY_METHOD_AUTO }

> usersetting.cpp:25
> +#include <QDebug>
> +
> +NetworkManager::UserSettingPrivate::UserSettingPrivate()

User setting was introduced with NM 1.8. Here should be ifdef to check the version and define NM_SETTING_USER_DATA in case the version is older.

> usersetting.cpp:27
> +NetworkManager::UserSettingPrivate::UserSettingPrivate()
> +    : name(NM_SETTING_IP_TUNNEL_SETTING_NAME)
> +{ }

Should be NM_SETTING_USER_SETTING_NAME

> usersetting.h:35
> +/**
> + * Represents generic setting
> + */

Represents user setting

> usersetting.h:52
> +    NMStringMap data() const;
> +
> +

No reason for such a big space.

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

To: pranavgade, jgrulich
Cc: ngraham, kde-frameworks-devel, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20181129/43ce5085/attachment-0001.html>