----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105324/#review14997 -----------------------------------------------------------
backends/NetworkManager/connectiondbus.cpp <http://git.reviewboard.kde.org/r/105324/#comment11791> Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty. I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE. libs/ui/connection.ui <http://git.reviewboard.kde.org/r/105324/#comment11796> If possible could you keep the order of items in increase order? This is row 2, it should go after row 1, not before row 0. libs/ui/connection.ui <http://git.reviewboard.kde.org/r/105324/#comment11797> You should use KComboBox instead of QComboBox. libs/ui/connection.ui <http://git.reviewboard.kde.org/r/105324/#comment11799> If this is a system connection and the user does not have permission to edit it this should be changed to false in the source code, no? libs/ui/connectionwidget.cpp <http://git.reviewboard.kde.org/r/105324/#comment11792> It is not advisabled to include entire Qt modules into the source code. Please add only the classes you need here. libs/ui/connectionwidget.cpp <http://git.reviewboard.kde.org/r/105324/#comment11798> Code style, you should use { } like this: if (...) { ... } else { ... } There are other places that needs the same code style change. - Lamarque Vieira Souza On June 22, 2012, 9:54 a.m., Lukáš Tinkl wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105324/ > ----------------------------------------------------------- > > (Updated June 22, 2012, 9:54 a.m.) > > > Review request for Network Management and Lamarque Vieira Souza. > > > Description > ------- > > NetworkManager has had support for FirewallD [1] and Network Zones [2] > since [3]. > > This patch adds one combo box that enables to change the network zone. > This box is visible only when FirewallD is running. > > [1] https://fedorahosted.org/firewalld/ > [2] https://fedoraproject.org/wiki/Features/network-zones > [3] > http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701 > > (on behalf of Jiri Popelka <[email protected]>) > > > Diffs > ----- > > backends/NetworkManager/connectiondbus.cpp 4649a19 > libs/internals/connection.h 485b810 > libs/internals/connection.cpp 67f31bc > libs/ui/connection.ui 7b9f873 > libs/ui/connectionwidget.h aac7050 > libs/ui/connectionwidget.cpp f67f157 > > Diff: http://git.reviewboard.kde.org/r/105324/diff/ > > > Testing > ------- > > > Thanks, > > Lukáš Tinkl > >
_______________________________________________ kde-networkmanager mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-networkmanager
