D17317: match and tc setting

This revision was automatically updated to reflect the committed changes. Closed by commit R282:e95bb9bdf98c: match and tc setting (authored by jgrulich). REPOSITORY R282 NetworkManagerQt CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46826&id=46827 REVISION DETAIL https:/

D17317: match and tc setting

jgrulich accepted this revision. This revision is now accepted and ready to land. REVISION DETAIL https://phabricator.kde.org/D17317 To: pranavgade, jgrulich Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

pranavgade updated this revision to Diff 46826. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46825&id=46826 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settings/matchsettingtest.cpp autotests/settings/

D17317: match and tc setting

jgrulich added a comment. It again doesn't apply, rebase it to current master. REVISION DETAIL https://phabricator.kde.org/D17317 To: pranavgade, jgrulich Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

pranavgade updated this revision to Diff 46825. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46824&id=46825 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settings/matchsettingtest.cpp autotests/settings/

D17317: match and tc setting

jgrulich added a comment. In D17317#371010 , @pranavgade wrote: > In D17317#371009 , @jgrulich wrote: > > > The tcsettingtest is failing. > > > Why, exactly? Because it's broken in num

D17317: match and tc setting

pranavgade added a comment. In D17317#371009 , @jgrulich wrote: > The tcsettingtest is failing. Why, exactly? REVISION DETAIL https://phabricator.kde.org/D17317 To: pranavgade, jgrulich Cc: kde-frameworks-devel, michaelh, ngraham, brun

D17317: match and tc setting

jgrulich added a comment. The tcsettingtest is failing. REVISION DETAIL https://phabricator.kde.org/D17317 To: pranavgade, jgrulich Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

pranavgade updated this revision to Diff 46824. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46792&id=46824 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settings/matchsettingtest.cpp autotests/settings/

D17317: match and tc setting

jgrulich added a comment. Can you please rebase your changes on top of the current master? I did some changes there and the patch is not applicable. REVISION DETAIL https://phabricator.kde.org/D17317 To: pranavgade, jgrulich Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

pranavgade updated this revision to Diff 46792. pranavgade marked an inline comment as done. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46789&id=46792 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settin

D17317: match and tc setting

jgrulich added a comment. And you submitted only changes on top of your changes. INLINE COMMENTS > tcsettingtest.cpp:92 > +// Here the maps should have same keys so compare > QVariantMaps as we do now > +QCOMPARE(listMap, listMap1); > +

D17317: match and tc setting

pranavgade updated this revision to Diff 46789. pranavgade marked 4 inline comments as done. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46786&id=46789 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settin

D17317: match and tc setting

jgrulich added inline comments. INLINE COMMENTS > tcsettingtest.cpp:92 > +if (comparedvals == map.size()) { > +comparedMaps++; > +} You still don't compare the values. > jgrulich wrote in setting.cpp:33 > Same here, should be NM 1.14.0. NM 1.14.0 is need

D17317: match and tc setting

pranavgade updated this revision to Diff 46786. pranavgade marked 3 inline comments as done. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46785&id=46786 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settin

D17317: match and tc setting

pranavgade updated this revision to Diff 46785. pranavgade marked 5 inline comments as done. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46761&id=46785 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settin

D17317: match and tc setting

jgrulich added inline comments. INLINE COMMENTS > matchsettingtest.cpp:29 > + > +#if !NM_CHECK_VERSION(1, 12, 0) > +#define NM_SETTING_MATCH_INTERFACE_NAME"interface-name" NM 1.14.0, which is not released yet. > jgrulich wrote in tcsettingtest.cpp:74 > Something like: > > NMVariantMapLis

D17317: match and tc setting

jgrulich added inline comments. INLINE COMMENTS > tcsettingtest.cpp:74 > +while (it != map.constEnd()) { > +QCOMPARE(it.value(), map1.value(it.key())); > +++it; Something like: NMVariantMapList list = it.value(); NMVariantMapList list1 = map1.value(it.key()); QCOMPARE(

D17317: match and tc setting

pranavgade added inline comments. INLINE COMMENTS > jgrulich wrote in tcsettingtest.cpp:69 > 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 va

D17317: match and tc setting

pranavgade updated this revision to Diff 46761. pranavgade marked 9 inline comments as done. REPOSITORY R282 NetworkManagerQt CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46724&id=46761 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settin

D17317: match and tc setting

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

D17317: match and tc setting

jgrulich added a comment. Why did you add my test for tun setting? REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D17317 To: pranavgade, jgrulich Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

pranavgade created this revision. pranavgade added a reviewer: jgrulich. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. pranavgade requested review of this revision. REVISION SUMMARY Added match and tc setting according to: https://developer.gnome.org/Netw