[Marble-devel] Review Request: Marble: Geonames.org support

[Bastian Holst]

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


Well looks good in general, but you should have a look at the coding style again.
I havn't done any testing, but I will do it when I got my own patch compiling ;).


/trunk/KDE/kdeedu/marble/src/lib/FileLoader.cpp
<http://reviewboard.kde.org/r/4006/#comment5431>

    I'm still not sure, why you do that. We could basically support any file suffix/type.



/trunk/KDE/kdeedu/marble/src/lib/FileLoader.cpp
<http://reviewboard.kde.org/r/4006/#comment5430>

    Why do you (re)move spaces here?



/trunk/KDE/kdeedu/marble/src/lib/FileLoader.cpp
<http://reviewboard.kde.org/r/4006/#comment5432>

    This line look strange, at least on reviewboard here.
    "}" missing?
    no indentation?



/trunk/KDE/kdeedu/marble/src/lib/FileLoader.cpp
<http://reviewboard.kde.org/r/4006/#comment5433>

    again, the indentation looks strange here, too ...
    or is it reviewboard again


- Bastian


On 2010-05-20 21:53:36, hjain wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4006/
> -----------------------------------------------------------
> 
> (Updated 2010-05-20 21:53:36)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> This patch is to use Geonames.org support for city placemarks. This patch also gives kml support for asciiname. Presently Marble is not generating cache for KML file. This issue is also solved.
> 
> Note:- It seems that there are more cities shown by default if you see the earth from far away
> 
> 
> This addresses bugs 232261 and 232450.
>     https://bugs.kde.org/show_bug.cgi?id=232261
>     https://bugs.kde.org/show_bug.cgi?id=232450
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/data/placemarks/HOWTO-cities.txt 1127478 
>   /trunk/KDE/kdeedu/marble/data/placemarks/baseplacemarks.cache UNKNOWN 
>   /trunk/KDE/kdeedu/marble/data/placemarks/boundaryplacemarks.cache UNKNOWN 
>   /trunk/KDE/kdeedu/marble/data/placemarks/cityplacemarks.cache UNKNOWN 
>   /trunk/KDE/kdeedu/marble/data/placemarks/elevplacemarks.cache UNKNOWN 
>   /trunk/KDE/kdeedu/marble/data/placemarks/otherplacemarks.cache UNKNOWN 
>   /trunk/KDE/kdeedu/marble/src/lib/FileLoader.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleDirs.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/PlacemarkInfoDialog.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/PlacemarkLayout.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/PlacemarkManager.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature.h 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature_p.h 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPlacemark.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPlacemark_p.h 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlRoleTagHandler.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/tools/asc2kml/asc2kml.cpp 1127478 
> 
> Diff: http://reviewboard.kde.org/r/4006/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> hjain
> 
>