D17317: match and tc setting

2018-12-04 Thread Jan Grulich
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=46827 REVISION DETAIL

D17317: match and tc setting

2018-12-04 Thread Jan Grulich
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

2018-12-04 Thread Pranav Gade
pranavgade updated this revision to Diff 46826. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46825=46826 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settings/matchsettingtest.cpp

D17317: match and tc setting

2018-12-04 Thread Jan Grulich
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

2018-12-04 Thread Pranav Gade
pranavgade updated this revision to Diff 46825. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46824=46825 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settings/matchsettingtest.cpp

D17317: match and tc setting

2018-12-04 Thread Jan Grulich
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

D17317: match and tc setting

2018-12-04 Thread Pranav Gade
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,

D17317: match and tc setting

2018-12-04 Thread Jan Grulich
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

2018-12-04 Thread Pranav Gade
pranavgade updated this revision to Diff 46824. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17317?vs=46792=46824 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt autotests/settings/matchsettingtest.cpp

D17317: match and tc setting

2018-12-03 Thread Jan Grulich
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

2018-12-03 Thread Pranav Gade
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=46792 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt

D17317: match and tc setting

2018-12-03 Thread Jan Grulich
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

2018-12-03 Thread Pranav Gade
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=46789 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt

D17317: match and tc setting

2018-12-03 Thread Jan Grulich
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

D17317: match and tc setting

2018-12-03 Thread Pranav Gade
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=46786 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt

D17317: match and tc setting

2018-12-03 Thread Pranav Gade
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=46785 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES autotests/settings/CMakeLists.txt

D17317: match and tc setting

2018-12-03 Thread Jan Grulich
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: > >

D17317: match and tc setting

2018-12-02 Thread Jan Grulich
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());

D17317: match and tc setting

2018-12-02 Thread Pranav Gade
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

D17317: match and tc setting

2018-12-02 Thread Pranav Gade
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=46761 REVISION DETAIL https://phabricator.kde.org/D17317 AFFECTED FILES

D17317: match and tc setting

2018-12-02 Thread 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 > + > +

D17317: match and tc setting

2018-12-02 Thread Jan Grulich
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

2018-12-02 Thread Pranav Gade
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: