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

[Torsten Rahn]

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


Harshit, I like the patch. But it still needs a lot of work. But that's not your fault. The topic is pretty complex and I think you made a very good start given the complexity of the issue and given the complexity of the code. Most important is that we need some discussion regarding the ascii-stuff. 
And a general advice: If you don't understand some lines of code. But if you think that something goes wrong in those lines. And if you think that the code is the root of the problem and if you therefore decide to change it then please make a comment that the original code doesn't seem to make sense to you. Otherwise you might introduce regressions and bugs which don't get spotted by others. We might either decide in that situation that it's a bug or that it needs better source code documentation :-)


- Torsten


On 2010-05-17 14:46:27, hjain wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4006/
> -----------------------------------------------------------
> 
> (Updated 2010-05-17 14:46:27)
> 
> 
> 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/MarblePlacemarkModel.h 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/MarblePlacemarkModel.cpp 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/PlacemarkInfoDialog.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.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/KmlAsciiNameTagHandler.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlAsciiNameTagHandler.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlElementDictionary.h 1127478 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/kml/KmlElementDictionary.cpp 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
> 
>