Review Request 128093: Make network manager version checks in runtime (to avoid compile vs. run-time discrepancies)
Lamarque Souza
lamarque at kde.org
Tue Jun 7 16:34:08 UTC 2016
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128093/#review96260
-----------------------------------------------------------
src/device.cpp (line 125)
<https://git.reviewboard.kde.org/r/128093/#comment65045>
This should be checkVersion(1, 0, 6).
src/manager.h (line 122)
<https://git.reviewboard.kde.org/r/128093/#comment65046>
Use tag @deprecated here to indicate that.
src/manager.h (line 137)
<https://git.reviewboard.kde.org/r/128093/#comment65047>
Same here.
src/manager.h (line 224)
<https://git.reviewboard.kde.org/r/128093/#comment65048>
Please fix my spelling error :-)
s/othervise/otherwise/
src/manager.h (line 293)
<https://git.reviewboard.kde.org/r/128093/#comment65049>
@deprecated
src/manager.h (line 303)
<https://git.reviewboard.kde.org/r/128093/#comment65050>
@deprecated
src/manager.h (line 420)
<https://git.reviewboard.kde.org/r/128093/#comment65051>
Well, the correct way to do that is adding the Q_DECL_DEPRECATED modified after NETWORKMANAGER_QT_EXPORT macro. However, that will issue several compiler warnings during compilation, and some people my not like that, me doesn't. Maybe there is a way to suppress those warning messages using cmake.
src/manager.cpp (line 298)
<https://git.reviewboard.kde.org/r/128093/#comment65052>
This line and the next can be removed.
src/settings.cpp (line 143)
<https://git.reviewboard.kde.org/r/128093/#comment65053>
Shouldn't you call iface.ReloadConnections() here?
src/settings/gsmsetting.cpp (line 24)
<https://git.reviewboard.kde.org/r/128093/#comment65055>
This line looks wrong. There should be no negation in it.
src/settings/infinibandsetting.cpp (line 24)
<https://git.reviewboard.kde.org/r/128093/#comment65056>
Same here.
src/settings/teamsetting.cpp (line 25)
<https://git.reviewboard.kde.org/r/128093/#comment65057>
Here too.
src/settings/vlansetting.cpp (line 24)
<https://git.reviewboard.kde.org/r/128093/#comment65058>
Same here.
src/settings/wirelesssetting.cpp (line 24)
<https://git.reviewboard.kde.org/r/128093/#comment65059>
Same here.
src/vlandevice.cpp (line 83)
<https://git.reviewboard.kde.org/r/128093/#comment65054>
Use Q_NULLPTR instead of nullptr.
- Lamarque Souza
On June 6, 2016, 4 p.m., Palo Kisa wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128093/
> -----------------------------------------------------------
>
> (Updated June 6, 2016, 4 p.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 35d4282
> 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/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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20160607/8c22f4f0/attachment-0001.html>
More information about the kde-networkmanager
mailing list