D6687: Adding support to ipv*.route-metric
lvsouza accepted this revision. This revision is now accepted and ready to land. REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D6687 To: pvillani, jgrulich, lvsouza Cc: #frameworks
D6687: Adding support to ipv*.route-metric
jgrulich added a comment.
Looks good to me now, it looked to me good even before, but didn't read
carefully the documentation. Lamarque, do you see anything else?
INLINE COMMENTS
> ipv4setting.cpp:455
>
> +if(routeMetric() >= 0) {
> +setting.insert(QLatin1String(NMQT_SETTING_IP4_CONFIG_ROUTE_METRIC),
> routeMetric());
Missing space between if and bracket.
REPOSITORY
R282 NetworkManagerQt
REVISION DETAIL
https://phabricator.kde.org/D6687
To: pvillani, jgrulich, lvsouza
Cc: #frameworks
D6687: Adding support to ipv*.route-metric
pvillani added a comment. In https://phabricator.kde.org/D6687#125529, @lvsouza wrote: > The summary says this patch adds route metric support to IPv4 too, but no IPv4 file is touched by this patch. Have you missed the IPv4 changes? Added the missing files. Aboute your comment suggestion, I preferred to change the code and accept the value 0, so that it works as specified by the documentation. In the case of ipv6, if the value 0 is set, it will be automatically changed to 1024 by NetworkManager. REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D6687 To: pvillani, jgrulich, lvsouza Cc: #frameworks
D6687: Adding support to ipv*.route-metric
pvillani updated this revision to Diff 16836. pvillani added a comment. Adding missing files and making possible to set route-metric as 0, as described in NetworkManager documentation. REPOSITORY R282 NetworkManagerQt CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6687?vs=16697=16836 REVISION DETAIL https://phabricator.kde.org/D6687 AFFECTED FILES TODO autotests/settings/ipv4settingtest.cpp autotests/settings/ipv6settingtest.cpp src/settings/ipv4setting.cpp src/settings/ipv4setting.h src/settings/ipv4setting_p.h src/settings/ipv6setting.cpp src/settings/ipv6setting.h src/settings/ipv6setting_p.h To: pvillani, jgrulich, lvsouza Cc: #frameworks
D6687: Adding support to ipv*.route-metric
lvsouza requested changes to this revision.
lvsouza added a comment.
This revision now requires changes to proceed.
The summary says this patch adds route metric support to IPv4 too, but no
IPv4 file is touched by this patch. Have you missed the IPv4 changes?
INLINE COMMENTS
> ipv6setting.cpp:412
>
> +if(routeMetric() > 0){
> +setting.insert(QLatin1String(NMQT_SETTING_IP6_CONFIG_ROUTE_METRIC),
> routeMetric());
You could have added a comment stating that according to NetworkManager's
documentation zero is not a possible value for IPv6's route metric (the value
is automatically changed to 1024 when trying to set it to zero).
REPOSITORY
R282 NetworkManagerQt
REVISION DETAIL
https://phabricator.kde.org/D6687
To: pvillani, jgrulich, lvsouza
Cc: #frameworks
D6687: Adding support to ipv*.route-metric
jgrulich accepted this revision.
jgrulich added a comment.
This revision is now accepted and ready to land.
Looks good, thank you so much.
INLINE COMMENTS
> ipv6setting.cpp:412
>
> +if(routeMetric() > 0){
> +setting.insert(QLatin1String(NMQT_SETTING_IP6_CONFIG_ROUTE_METRIC),
> routeMetric());
Missing space between if and bracket.
REPOSITORY
R282 NetworkManagerQt
REVISION DETAIL
https://phabricator.kde.org/D6687
To: pvillani, jgrulich, lvsouza
Cc: #frameworks
D6687: Adding support to ipv*.route-metric
pvillani requested review of this revision. REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D6687 To: pvillani, jgrulich, lvsouza Cc: #frameworks
D6687: Adding support to ipv*.route-metric
pvillani added a comment. Done REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D6687 To: pvillani, jgrulich, lvsouza Cc: #frameworks
D6687: Adding support to ipv*.route-metric
pvillani updated this revision to Diff 16697. pvillani retitled this revision from "Adding support to ipv4.route-metric" to "Adding support to ipv*.route-metric". pvillani edited the summary of this revision. pvillani edited the test plan for this revision. pvillani added a comment. Adding support to piv6.route-metrics. TODO updated. REPOSITORY R282 NetworkManagerQt CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6687?vs=16665=16697 REVISION DETAIL https://phabricator.kde.org/D6687 AFFECTED FILES TODO autotests/settings/ipv6settingtest.cpp src/settings/ipv6setting.cpp src/settings/ipv6setting.h src/settings/ipv6setting_p.h To: pvillani, jgrulich, lvsouza Cc: #frameworks
