[Marble-devel] Review Request: Code cleanup: Move some static methods from TileLoaderHelper directly to GeoSceneTexture and use TileId as parameter.
Torsten Rahn
rahn at kde.org
Sat May 1 13:35:57 CEST 2010
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3588/#review5350
-----------------------------------------------------------
trunk/KDE/kdeedu/marble/src/lib/ServerLayout.h
<http://reviewboard.kde.org/r/3588/#comment5070>
Include guards are named MARBLE_STORAGELAYOUT_H while the class is named ServerLayout already. I don't have anything against a name change but was there a reason to change it to "server"layout instead of "storage"layout? I'm just curious.
trunk/KDE/kdeedu/marble/src/lib/ServerLayout.h
<http://reviewboard.kde.org/r/3588/#comment5068>
Hm, I feel there is a bit of documentation missing here. What is the Url provided by the first parameter? What is the Url returned? Maybe something like serverUrl, or rootDirUrl, or something even better?
trunk/KDE/kdeedu/marble/src/lib/ServerLayout.cpp
<http://reviewboard.kde.org/r/3588/#comment5069>
What does "prototype" mean in this case? :-)
- Torsten
On 2010-04-30 10:36:41, Bernhard Beschow wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3588/
> -----------------------------------------------------------
>
> (Updated 2010-04-30 10:36:41)
>
>
> Review request for marble.
>
>
> Summary
> -------
>
> This patch represents milestone 1 of a series of patches to add more (server-side) storage layouts. In this series of patches, an abstract base class will be used to create download URLs, allowing for easy addition of server-side storage layouts. See [1] for all patches.
>
> [1] http://github.com/shentok/marble/commits/custom-storage
>
>
> Diffs
> -----
>
> trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1121091
> trunk/KDE/kdeedu/marble/src/lib/ServerLayout.h PRE-CREATION
> trunk/KDE/kdeedu/marble/src/lib/ServerLayout.cpp PRE-CREATION
> trunk/KDE/kdeedu/marble/src/lib/StackedTileLoader.cpp 1121091
> trunk/KDE/kdeedu/marble/src/lib/TileLoader.cpp 1121091
> trunk/KDE/kdeedu/marble/src/lib/TileLoaderHelper.h 1121091
> trunk/KDE/kdeedu/marble/src/lib/TileLoaderHelper.cpp 1121091
> trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/dgml/DgmlStorageLayoutTagHandler.cpp 1121091
> trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.h 1121091
> trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.cpp 1121091
> trunk/KDE/kdeedu/marble/src/tilecreator/CMakeLists.txt 1121091
>
> Diff: http://reviewboard.kde.org/r/3588/diff
>
>
> Testing
> -------
>
> Works for me, please test.
>
>
> Thanks,
>
> Bernhard
>
>
More information about the Marble-devel
mailing list