> On June 7, 2016, 4:34 p.m., Lamarque Souza wrote: > > src/settings/gsmsetting.cpp, line 24 > > <https://git.reviewboard.kde.org/r/128093/diff/4/?file=467926#file467926line24> > > > > This line looks wrong. There should be no negation in it. > > Palo Kisa wrote: > Why? Was the pre-existing logic wrong? > > Lamarque Souza wrote: > Yes, that pre-existing logic was wrong. > > Palo Kisa wrote: > Shouldn't that be fixed in separate commit rather than hidden in this > unrelated one? > > Lamarque Souza wrote: > Yes, it can be fixed in a separated patch. > > Palo Kisa wrote: > While changing the logic I've realized, that the version check negation > seems to be correct -> consider this commit > https://github.com/NetworkManager/NetworkManager/commit/d0b05b34d5f1ad8f52a15fed48cc2c02d2251145 > . Are you sure, you want to change it? > > Jan Grulich wrote: > It's correct, there is no need to change it. > > Lamarque Souza wrote: > Ok, the negation is right but I think we should do what the comment in > https://github.com/NetworkManager/NetworkManager/commit/d0b05b34d5f1ad8f52a15fed48cc2c02d2251145 > says and use the new headers instead of manually #defining the strings we > need. So change these lines to: > > #if NM_CHECK_VERSION(1, 0, 0) > #include <libnm/NetworkManager.h> > #else > #include <nm-setting-gsm.h> > #endif
You are seeing it wrong: 1. We are not (re)defining anything that the NetworkManager provides on its own. We are just defining the deprecated&dropped properties, see e.g. this commit https://github.com/NetworkManager/NetworkManager/commit/a5ac95ca4bab34cfafd94e3f785bbe51e9b0b223 2. All src/setting/*setting.cpp are including src/setting/*setting.h -> src/setting/setting.h -> \<libnm/NetworkManager.h> - Palo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128093/#review96260 ----------------------------------------------------------- On June 14, 2016, 6:36 a.m., Palo Kisa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128093/ > ----------------------------------------------------------- > > (Updated June 14, 2016, 6:36 a.m.) > > > Review request for Network Management, Jan Grulich and Lamarque Souza. > > > Repository: networkmanager-qt > > > Description > ------- > > https://bugs.kde.org/show_bug.cgi?id=362736 > > > Diffs > ----- > > CMakeLists.txt bbd5868 > src/CMakeLists.txt 0470d8f > src/accesspoint.h 51a1b2f > src/accesspoint.cpp 62cbc6c > src/accesspoint_p.h 521f8ce > src/activeconnection.h 7516ad6 > src/activeconnection.cpp e833f51 > src/activeconnection_p.h 79a0055 > src/connection.h 5dd7a16 > src/connection.cpp 57c5aa0 > src/connection_p.h fb0b90b > src/device.h f465f78 > src/device.cpp a3f6fad > src/device_p.h 4d7bcb6 > src/ipconfig.h 24de6d2 > src/ipconfig.cpp d1316fc > src/manager.h 0f36ba0 > src/manager.cpp bd8752c > src/manager_p.h 8de789d > src/settings.h 76cdd8d > src/settings.cpp a15165f > src/settings/bondsetting.cpp 3d52611 > src/settings/bridgesetting.cpp 05ce74c > src/settings/connectionsettings.h 2dd00af > src/settings/connectionsettings.cpp b62641e > src/settings/connectionsettings_p.h d337c99 > src/settings/gsmsetting.cpp b13274b > src/settings/infinibandsetting.h c0c854e > src/settings/infinibandsetting.cpp 6ced9b1 > src/settings/infinibandsetting_p.h 238cd76 > src/settings/setting.cpp ba75623 > src/settings/teamsetting.cpp 60ff218 > src/settings/tunsetting.cpp ead153c > src/settings/vlansetting.cpp 5c39dc9 > src/settings/wirelesssetting.cpp aa143c4 > src/settings_p.h aec5423 > src/vlandevice.h cec84ea > src/vlandevice.cpp 029cab0 > src/vlandevice_p.h 83b6b6f > src/wimaxdevice.cpp 468f50c > src/wirelessdevice.h 65313a7 > src/wirelessdevice.cpp 9439606 > > Diff: https://git.reviewboard.kde.org/r/128093/diff/ > > > Testing > ------- > > It's a relatively big change... so everything needs to be double checked. > > > File Attachments > ---------------- > > 0001-Make-the-networkmanager-version-checks-in-runtime.patch > > https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/54f5a458-701f-4590-b9b4-21ea0c04f682__0001-Make-the-networkmanager-version-checks-in-runtime.patch > > > Thanks, > > Palo Kisa > >
_______________________________________________ kde-networkmanager mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-networkmanager
