Review Request: added support for collors and size on fifteenpuzzle.

[Aaron Seigo]

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

Ship it!


there are some formatting issues, but otherwise this is ready to go. if you could just do one more pass through your code and tidy up the whitespace and what not then please commit this to trunk (don't need to do another upload to review board afaic) .... thanks again for the great work and cool new features!


/trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteen.cpp
<http://reviewboard.kde.org/r/1706/#comment1801>

    opening brace on line by itself



/trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteen.cpp
<http://reviewboard.kde.org/r/1706/#comment1803>

    while (--numSteps) {



/trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteen.cpp
<http://reviewboard.kde.org/r/1706/#comment1802>

    please stick to the style in the rest of the code for this widget, which is the kdelibs style (which Plasma tends to use in general), making this:
    
    if (orientation) {
    ..
    } else {
    ...
    }



/trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzle.cpp
<http://reviewboard.kde.org/r/1706/#comment1804>

    consider using BoardSize and BoardColor, as the other config values are AllCapsLikeThatAlready ;)


- Aaron


On 2009-09-24 15:05:37, Tomaz Canabrava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1706/
> -----------------------------------------------------------
> 
> (Updated 2009-09-24 15:05:37)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> colors are now configurable,
> and number of pieces too.
> everything is saved and restored.
> shuffle now also shows a bit of movement while shuffling.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/CMakeLists.txt 1027654 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/images/blanksquare.svg PRE-CREATION 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteen.h 1027654 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteen.cpp 1027654 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzle.cpp 1027654 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzleConfig.h 1027654 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzleConfig.cpp 1027654 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzleConfig.ui 1027654 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/piece.h 1027654 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/piece.cpp 1027654 
> 
> Diff: http://reviewboard.kde.org/r/1706/diff
> 
> 
> Testing
> -------
> 
> everything looks ok.
> 
> 
> Thanks,
> 
> Tomaz
> 
>