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

[1989.gaurav at gmail.com]

> On 2010-08-15 16:13:56, Thibaut Gridel wrote:
> > 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.

Corrected the coordinate thing, It was due to selecting wrong unit

Removing all commented code which is not of use for this patch
Removed all changed in GeoDataTreeModel


> On 2010-08-15 16:13:56, Thibaut Gridel wrote:
> > /trunk/KDE/kdeedu/marble/src/QtMainWindow.h, line 108
> > <http://reviewboard.kde.org/r/5034/diff/3/?file=33976#file33976line108>
> >
> >     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...)

removed all comments.
How Git would be useful in this case? So I can think of moving to git again ;)


> On 2010-08-15 16:13:56, Thibaut Gridel wrote:
> > /trunk/KDE/kdeedu/marble/src/lib/GeoDataTreeModel.cpp, line 176
> > <http://reviewboard.kde.org/r/5034/diff/3/?file=33982#file33982line176>
> >
> >     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.

I don't want all placemark data like population ares etc. So I thought to override it for bookmark. Removing for this patch, but please suggest me what is the better way of doing this if I want only name and desciption. Because I dont see any use of other fields in context of bookmrk.


- 1989gaurav


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


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/d23358db/attachment.htm