[Marble-devel] Re: Review Request: Map Creation Wizard

[Torsten Rahn]

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


Code looks excellent to me. Only thing that looks a bit strange to me is the basetile vs. source image thing. You are querying the base tile and later on do something with the source image when you just should query and work with the source image instead. I don't understand what you are doing there. Can you explain? :-)
What example did you use to test your code? I still need to test it. 


trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp
<http://svn.reviewboard.kde.org/r/6070/#comment10013>

    Let's make this more motivational with this string:
    
    "Create your own new map for Marble using this wizard."



trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp
<http://svn.reviewboard.kde.org/r/6070/#comment10014>

    Hm, obviously you're trying to make the window modal here. I don't know why this doesn't work out of the box. Comparing the lines with the Qt Docs I can only see that in the Qt docs the wizard is instantiated on the stack and then "show()" being called. Maybe that provides the expected result?



trunk/KDE/kdeedu/marble/src/lib/MapWizard.h
<http://svn.reviewboard.kde.org/r/6070/#comment10015>

    why not put this into the private class? That's what the private class was made for :-)



trunk/KDE/kdeedu/marble/src/lib/MapWizard.h
<http://svn.reviewboard.kde.org/r/6070/#comment10016>

    please use "const QString&" for all parameters in a setter instead of "QString".
    Also in a public API a set of lots of QStrings isn't very instructional and extensible. But I realize that it's convenient in this case. 



trunk/KDE/kdeedu/marble/src/lib/MapWizard.h
<http://svn.reviewboard.kde.org/r/6070/#comment10017>

    See comment above



trunk/KDE/kdeedu/marble/src/lib/MapWizard.h
<http://svn.reviewboard.kde.org/r/6070/#comment10018>

    see above



trunk/KDE/kdeedu/marble/src/lib/MapWizard.cpp
<http://svn.reviewboard.kde.org/r/6070/#comment10020>

    hm, no this should not refer to the base tile. It should refer to the source image. From the source image the tile creator then will create the tiles.The source image is just placed inside the map themes directory. Have a look at the bluemarble theme how this thing works :-)



trunk/KDE/kdeedu/marble/src/lib/MapWizard.ui
<http://svn.reviewboard.kde.org/r/6070/#comment10019>

    Hm, you are surely referring to the "Source Image" right?
    After all the whole idea is that the user selects an image of the worldmap that gets cut into tiles by the tilecreator.
    


- Torsten


On 2010-12-08 13:40:07, Utku Aydin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6070/
> -----------------------------------------------------------
> 
> (Updated 2010-12-08 13:40:07)
> 
> 
> Review request for marble, Torsten Rahn and Dennis Nienhüser.
> 
> 
> Summary
> -------
> 
> Map wizard helps users create a new map easily. It queries the new map's details including base tile image and preview image.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/src/QtMainWindow.h 1202078 
>   trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp 1202078 
>   trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1202078 
>   trunk/KDE/kdeedu/marble/src/lib/MapViewWidget.ui 1202078 
>   trunk/KDE/kdeedu/marble/src/lib/MapWizard.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/lib/MapWizard.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/lib/MapWizard.ui PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/lib/MarbleThemeSelectView.h 1202078 
>   trunk/KDE/kdeedu/marble/src/lib/MarbleThemeSelectView.cpp 1202078 
> 
> Diff: http://svn.reviewboard.kde.org/r/6070/diff
> 
> 
> Testing
> -------
> 
> I tested the wizard in different situations and it doesn't let the user overwrite the existing maps. Also it warns the user properly if there is an invalid input.
> 
> 
> Screenshots
> -----------
> 
> Step One
>   http://svn.reviewboard.kde.org/r/6070/s/577/
> Step Two
>   http://svn.reviewboard.kde.org/r/6070/s/578/
> 
> 
> Thanks,
> 
> Utku
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20101208/85e36eaf/attachment.htm