Added proxy and user settings
ClosedPublic

Authored by pranavgade on Nov 28 2018, 10:13 AM.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
pranavgade created this revision.Nov 28 2018, 10:13 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 28 2018, 10:13 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
pranavgade requested review of this revision.Nov 28 2018, 10:13 AM
pranavgade updated this revision to Diff 46444.Nov 29 2018, 3:59 AM

Updated diff from prev commit->changes to latest commit->changes

jgrulich added inline comments.Nov 29 2018, 11:23 AM
src/settings/proxysetting.cpp
25 ↗(On Diff #46444)

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.

src/settings/proxysetting.h
35 ↗(On Diff #46444)

Represents proxy setting

52 ↗(On Diff #46444)

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 }

src/settings/usersetting.cpp
25 ↗(On Diff #46444)

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.

27 ↗(On Diff #46444)

Should be NM_SETTING_USER_SETTING_NAME

src/settings/usersetting.h
35 ↗(On Diff #46444)

Represents user setting

52 ↗(On Diff #46444)

No reason for such a big space.

pranavgade updated this revision to Diff 46467.Nov 29 2018, 1:47 PM
pranavgade marked 6 inline comments as done.

Updated code as required,
fixed a minor error in iptunnelsettings

jgrulich added inline comments.Nov 29 2018, 1:57 PM
src/settings/iptunnelsetting.cpp
35

This is an unrelated change, submit it in a different review, but thanks for spotting this.

src/settings/proxysetting.cpp
26

I said NM 1.6, not 1.16 and you need !NM_CHECK_VERSION(1, 6, 0).

src/settings/proxysetting.h
42

Coding style. It should be:

enum Mode {
....
}

src/settings/usersetting.cpp
26

Same here. It should be !NM_CHECK_VERSION(1, 8, 0).

pranavgade updated this revision to Diff 46469.Nov 29 2018, 2:14 PM

made some changes

pranavgade marked 3 inline comments as done.Nov 29 2018, 2:38 PM
pranavgade added inline comments.
src/settings/proxysetting.cpp
25 ↗(On Diff #46444)
pranavgade marked 3 inline comments as done.Nov 29 2018, 2:38 PM
cfeck added a subscriber: cfeck.Nov 29 2018, 2:41 PM

Could you please set the Repository field?

pranavgade set the repository for this revision to R282 NetworkManagerQt.Nov 29 2018, 2:42 PM
jgrulich added inline comments.Nov 29 2018, 4:13 PM
src/settings/proxysetting.cpp
38 ↗(On Diff #46444)

Default value is false.

src/settings/usersetting.cpp
95 ↗(On Diff #46444)

One empty line is enough.

pranavgade updated this revision to Diff 46488.Nov 29 2018, 4:21 PM
pranavgade marked 2 inline comments as done.
jgrulich accepted this revision.Nov 29 2018, 4:22 PM
This revision is now accepted and ready to land.Nov 29 2018, 4:22 PM