D17317: match and tc setting

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> matchsettingtest.cpp:43
> +
> +    map.insert(QLatin1String(NM_SETTING_MATCH_INTERFACE_NAME), interfaceName);
> +

This define won't exist for NM < 1.12.0

> tcsettingtest.cpp:58
> +
> +    map.insert(QLatin1String(NM_SETTING_TC_CONFIG_TFILTERS), QVariant::fromValue(tfilters));
> +    map.insert(QLatin1String(NM_SETTING_TC_CONFIG_QDISCS), QVariant::fromValue(qdiscs));

TC defines do not exist before NM 1.10.0

> tcsettingtest.cpp:69
> +    while (it != map.constEnd()) {
> +        QCOMPARE(it.value(), map1.value(it.key()));
> +        ++it;

I think that you cannot compare NMVariantMaps this way, that's why probably the test for IPv4 and IPv6 didn't fail when we swapped values for route-data and address-data. You will need to compare values inside the maps separately.

> matchsetting.h:27
> +
> +#include <QString>
> +

I think you don't need to include <QString>

> matchsetting.h:29
> +
> +#if !NM_CHECK_VERSION(1, 12, 0)
> +#define NM_SETTING_MATCH_SETTING_NAME      "match"

Move those defines out of the header file.

> tcsetting.cpp:115
> +    dbg.nospace() << NM_SETTING_TC_CONFIG_QDISCS << ": " << '\n';
> +    Q_FOREACH (const QVariantMap & qdiscs, setting.qdiscs()) {
> +        QVariantMap::const_iterator i = qdiscs.constBegin();

const QVariantMap &qdisc

> tcsetting.cpp:118
> +        while (i != qdiscs.constEnd()) {
> +        dbg.nospace() << i.key() << ": " << i.value() << '\n';
> +        }

Indent.

> tcsetting.cpp:122
> +    dbg.nospace() << NM_SETTING_TC_CONFIG_TFILTERS << ": " << '\n';
> +    Q_FOREACH (const QVariantMap & tfilters, setting.tfilters()) {
> +        QVariantMap::const_iterator i = tfilters.constBegin();

const QVariantMap &tfilter

> tcsetting.h:30
> +
> +#if !NM_CHECK_VERSION(1, 12, 0)
> +#define NM_SETTING_TC_CONFIG_SETTING_NAME    "tc"

TC defines are present since NM 1.10.0. Also move them out of the header file.

> tcsetting_p.h:25
> +#include <QString>
> +#include "../ipconfig.h"
> +

This include shouldn't be needed.

REPOSITORY
  R282 NetworkManagerQt

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

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/20181202/59ce4d73/attachment-0001.html>