Review Request 121430: change in main.qml

[Inge Wallin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121430/#review71701
-----------------------------------------------------------


Ok, here's a review of the trivialities of the patch: white space, naming, etc.  Since I'm not good at QML, I'll let somebody else review that part.


src/kanagramgame.h
<https://git.reviewboard.kde.org/r/121430/#comment49994>

    function name start with lower case



src/kanagramgame.h
<https://git.reviewboard.kde.org/r/121430/#comment49995>

    Spaces at the end of lines are generally frowned upon. I'm not that bothered, but let's keep to the standards. There are several places where this needs to be fixed.



src/kanagramgame.cpp
<https://git.reviewboard.kde.org/r/121430/#comment49996>

    Also here: function names start with lower case letters. (cut&paste bug?)



src/kanagramgame.h~
<https://git.reviewboard.kde.org/r/121430/#comment49993>

    This whole file should be removed. It's only a backup file from the editor.



src/mainwindow.cpp
<https://git.reviewboard.kde.org/r/121430/#comment49997>

    Strange indentation



src/ui/main.qml
<https://git.reviewboard.kde.org/r/121430/#comment49998>

    In general old code should be removed rather than commented out.  There may be a reason why you keep it, but since you don't add any comment, it's impossible to say.


- Inge Wallin


On Dec. 10, 2014, 1:54 p.m., Souvik  Das wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121430/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 1:54 p.m.)
> 
> 
> Review request for KDE Edu.
> 
> 
> Repository: kanagram
> 
> 
> Description
> -------
> 
> change in main.qml
> 
> 
> Diffs
> -----
> 
>   src/kanagram.kcfg 468e316528c413b8dc42d2e9d365b6fe63c63257 
>   src/kanagramgame.h e9564cfcc6a19a3ed5e63db5340cd19a760914d5 
>   src/kanagramgame.cpp e0b60866ef49af3f28161c72982b17c6865f88ef 
>   src/kanagramgame.h~ PRE-CREATION 
>   src/mainwindow.cpp e1432562cdaff022b0f863bfbe72bf4abddbbc65 
>   src/ui/icons/1player.png PRE-CREATION 
>   src/ui/icons/2player.png PRE-CREATION 
>   src/ui/main.qml 38408ae66772a578a45faeb92ff9828f7c174f2a 
> 
> Diff: https://git.reviewboard.kde.org/r/121430/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Souvik  Das
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20141210/b14ea1b7/attachment-0001.html>