[Kde-games-devel] Review Request 109827: Add custom game level
Albert Astals Cid
aacid at kde.org
Wed Apr 3 23:19:15 UTC 2013
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109827/#review30345
-----------------------------------------------------------
src/CMakeLists.txt
<http://git.reviewboard.kde.org/r/109827/#comment22613>
I'm with Jakob in that it'd be cool if you did not change the spacing of the file, it's taking me much more than i'd like to see what is new in this file and what is just spacing changes, maybe you can add only the new stuff here and do the spacing fixes separately (no need for review on that)?
src/customgame.ui
<http://git.reviewboard.kde.org/r/109827/#comment22612>
I have the feeling that this layout could be a bit nicer, have you tried right aligning the label? Or not making the checkbox span two rows? The space between the label and the spinner feels a bit weird to me
src/customgame.ui
<http://git.reviewboard.kde.org/r/109827/#comment22611>
Some tooltip and/or whatsthis won't hurt here
src/mainwindow.cpp
<http://git.reviewboard.kde.org/r/109827/#comment22610>
This creates a dialog every time we click on the settings but the dialog is not freed until the mainwindow goes again so if we click 1000 times in configure settings we have 1000 dialogs resident in memory, you may want to either reuse the same dialog or make sure the dialog gets deleted on close (by using exec() + delete or by setting the deleteOnClose flag on the dialog)
- Albert Astals Cid
On April 3, 2013, 2:40 p.m., Ashwin Rajeev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109827/
> -----------------------------------------------------------
>
> (Updated April 3, 2013, 2:40 p.m.)
>
>
> Review request for KDE Games and Albert Astals Cid.
>
>
> Description
> -------
>
> Custom game level where user can select size ( 3 - 11 ) and whether wrapping is needed.
>
> Feature: 221727
>
>
> Diffs
> -----
>
> doc/index.docbook b264e43
> src/CMakeLists.txt fbee0f6
> src/customgame.ui PRE-CREATION
> src/knetwalk.kcfg 19e6f97
> src/main.cpp 0113bcf
> src/mainwindow.h f5609e4
> src/mainwindow.cpp 7c1e47f
>
> Diff: http://git.reviewboard.kde.org/r/109827/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ashwin Rajeev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20130403/fb773e7f/attachment.html>
More information about the kde-games-devel
mailing list