[Marble-devel] Review Request: PositionTracking: export and private

[Dennis Nienhüser]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/5038/#review7171
-----------------------------------------------------------

Ship it!


Looks good. Some of the comments below address things you didn't touch, just moved.


/trunk/KDE/kdeedu/marble/src/lib/PositionTracking.cpp
<http://reviewboard.kde.org/r/5038/#comment7310>

    assert that m_document is not empty? (i know that it is not empty atm, just to be safe for refactoring)



/trunk/KDE/kdeedu/marble/src/lib/PositionTracking.cpp
<http://reviewboard.kde.org/r/5038/#comment7311>

    what about keeping the lineString as a member to avoid the static_casts above?



/trunk/KDE/kdeedu/marble/src/lib/PositionTracking_p.h
<http://reviewboard.kde.org/r/5038/#comment7312>

    remove



/trunk/KDE/kdeedu/marble/src/lib/PositionTracking_p.h
<http://reviewboard.kde.org/r/5038/#comment7313>

    m_document( 0 )


- Dennis


On 2010-08-15 21:34:08, Thibaut Gridel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/5038/
> -----------------------------------------------------------
> 
> (Updated 2010-08-15 21:34:08)
> 
> 
> Review request for marble, Dennis Nienhüser and Siddharth Srivastava.
> 
> 
> Summary
> -------
> 
> Export PositionTracking as it should, and thus make a Private to hold data.
> Also provide a setTrackVisible to toggle track display and a resetTrack to reset the content of the document.
> The CurrentLocationWidget is updated as a proof of concept, please comment on UI aspects ;)
> 
> This patch should be useful to both Torch and bryang, who make use of the track display...
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/CurrentLocationWidget.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/CurrentLocationWidget.ui 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleDataFacade.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleMap.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleWidget.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/PositionTracking.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/PositionTracking.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/PositionTracking_p.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/gps/PositionTracking.h 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/gps/PositionTracking.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/AdjustNavigation.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingLayer.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingModel.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/positionmarker/PositionMarker.cpp 1164098 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp 1164098 
> 
> Diff: http://reviewboard.kde.org/r/5038/diff
> 
> 
> Testing
> -------
> 
> Various toggles in the Current Location widget.
> 
> 
> Thanks,
> 
> Thibaut
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100823/f8db072c/attachment-0001.htm