[Marble-devel] Re: Review Request: Positionprovider-plugin using QtMobility
Dennis Nienhüser
earthwings at gentoo.org
Tue May 24 15:06:02 CEST 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101431/#review3474
-----------------------------------------------------------
Looks good from looking over the code.
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2935>
Is that include needed?
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2925>
Should be private
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2926>
Should be private
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2927>
I'd go with Stroustrup and use 0 instead of NULL, same below.
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2928>
Who owns m_source? Do we need to shut it down somehow?
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2929>
That's a bit little to show to the user, should be more descriptive
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2931>
Please use curly brackets even for one-liners.
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2930>
Please use curly brackets even for one-liners.
src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp
<http://git.reviewboard.kde.org/r/101431/#comment2932>
Please use curly brackets even for one-liners.
- Dennis
On May 24, 2011, 12:46 p.m., Daniel Marth wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101431/
> -----------------------------------------------------------
>
> (Updated May 24, 2011, 12:46 p.m.)
>
>
> Review request for Marble.
>
>
> Summary
> -------
>
> Adds a positionprovider-plugin based on the QtMobility.
>
>
> Diffs
> -----
>
> FindQtLocation.cmake PRE-CREATION
> src/plugins/positionprovider/CMakeLists.txt c07c8bc
> src/plugins/positionprovider/qtmobility/CMakeLists.txt PRE-CREATION
> src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.h PRE-CREATION
> src/plugins/positionprovider/qtmobility/QtPositionProviderPlugin.cpp PRE-CREATION
>
> Diff: http://git.reviewboard.kde.org/r/101431/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Daniel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20110524/dc2781aa/attachment.htm
More information about the Marble-devel
mailing list