D17342: team-port setting

Jan Grulich noreply at phabricator.kde.org
Wed Dec 5 15:41:30 GMT 2018


jgrulich added inline comments.

INLINE COMMENTS

> teamportsettingtest.cpp:97
> +
> +    QVariantMap map0;
> +    map0.insert(QLatin1String(NM_SETTING_TEAM_PORT_LINK_WATCHERS), QVariant::fromValue(linkWatchers));

Still weird, why don't you put link-watchers to the same map as above. You can then skip comparison if the key is "link-watchers" as you saw in ipv6settingtest for example.

> teamportsettingtest.cpp:120
> +            while (ite != map.constEnd()) {
> +                if (map_1.contains(ite.key())) {
> +                    comparedvals++;

Here you compare whether the maps have identical keys, which is correct, but you also have to compare the values.

> teamportsetting.cpp:44
> +    : Setting(Setting::TeamPort)
> +    , d_ptr(new TeamPortSettingPrivate())
> +{ }

Default property initialization is missing.

> teamportsetting.cpp:226
> +
> +    if (!sticky()) {
> +        setting.insert(QLatin1String(NM_SETTING_TEAM_PORT_STICKY), sticky());

This is wrong, default value is FALSE, which means you would skip this property if it's set.

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

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


More information about the Kde-frameworks-devel mailing list