[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/#review9252
-----------------------------------------------------------



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

    This lacks a shortcut "&".



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

    while "get" is deprecated as a prefix for getters it's even a bit more strange for void methods. Maybe "querySourceImageFile()" ?



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

    "None" is a suboptimal enum value. Enum values should always end on the last syllable of the type to avoid clashes in the future. So "None" should rather be "NoMap".
    And "type" isn't descriptive either. maybe you could pick a better name?



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

    We always use preincrement when possible, so it should be ++i.



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

    Does WMS specify an expiration date?



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

    const QString& ....



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

    This one should have a short cut ("&").



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

    This lacks a short cut. e.g. "&Create a New Map ..."


- Torsten


On 2010-12-14 21:27:33, Utku Aydin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6070/
> -----------------------------------------------------------
> 
> (Updated 2010-12-14 21:27:33)
> 
> 
> Review request for marble, Torsten Rahn and Dennis Nienhüser.
> 
> 
> Summary
> -------
> 
> Map wizard helps users create a new map easily.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/src/QtMainWindow.h 1206280 
>   trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp 1206280 
>   trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1206280 
>   trunk/KDE/kdeedu/marble/src/lib/MapViewWidget.ui 1206280 
>   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 1206280 
>   trunk/KDE/kdeedu/marble/src/lib/MarbleThemeSelectView.cpp 1206280 
>   trunk/KDE/kdeedu/marble/src/marble.kcfg 1206280 
>   trunk/KDE/kdeedu/marble/src/marble_part.h 1206280 
>   trunk/KDE/kdeedu/marble/src/marble_part.cpp 1206280 
> 
> 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/20101214/63c7bb6d/attachment-0001.htm