[Marble-devel] Review Request: Bookmark Feature : Icons, Coordinate in bookmarks.kml and Edit Bookmark

[Thibaut Gridel]

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


I saved new bookmarks and loaded the bookmark file to see where they are... The Placemarks are at completely bogus places, there must be something wrong in their coordinates...
Please do not add code that is commented out it's harder to read the patch, and you should not commit those anyway.
I think you should also remove geodatatreemodel changes which are not ready yet.


/trunk/KDE/kdeedu/marble/src/QtMainWindow.h
<http://reviewboard.kde.org/r/5034/#comment7173>

    Please don't commit commented out code, just remove it and show it again when it is ready for review. (This is when git starts to become useful...)



/trunk/KDE/kdeedu/marble/src/lib/GeoDataTreeModel.cpp
<http://reviewboard.kde.org/r/5034/#comment7172>

    This sounds a bad idea to override the values of the column content for placemark just because that placemark happens to be a "bookmark placemark"
    
    What you could do instead is to use extra columns if the placemark is a bookmark, but not remove the existing ones.


- Thibaut


On 2010-08-15 13:59:02, 1989gaurav wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/5034/
> -----------------------------------------------------------
> 
> (Updated 2010-08-15 13:59:02)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> Added Icons in Bookmark menu.
> Added GeoDataCoordinate in bookmark.kml file along with lookAt.
> Added ExtendedData Tag writer to write in bookmark.kml file.
> Adding Edit Bookmark Feature along with some modification in GeoDataTreeModel ( In Process )
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/QtMainWindow.h 1163977 
>   /trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/BookmarkInfoDialog.cpp 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/BookmarkManager.h 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/BookmarkManager.cpp 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/GeoDataTreeModel.cpp 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleWidget.h 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleWidget.cpp 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataExtendedData.h 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataExtendedData.cpp 1163977 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/writers/kml/KmlDataTagWriter.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/writers/kml/KmlDataTagWriter.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/writers/kml/KmlExtendedDataTagWriter.h PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/writers/kml/KmlExtendedDataTagWriter.cpp PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/lib/geodata/writers/kml/KmlPlacemarkTagWriter.cpp 1163977 
>   /trunk/KDE/kdeedu/marble/src/marble.qrc 1163977 
> 
> Diff: http://reviewboard.kde.org/r/5034/diff
> 
> 
> Testing
> -------
> 
> Running
> 
> 
> Thanks,
> 
> 1989gaurav
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100815/55b36a04/attachment.htm