Review Request: Achievements 10: icons, score

[Laszlo Papp]

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



engine/achievement.h
<http://git.reviewboard.kde.org/r/102476/#comment5386>

    s/achievemnt\./achievement :p



engine/achievement.cpp
<http://git.reviewboard.kde.org/r/102476/#comment5388>

    I would personally put a line break after each separate if statement as it is done in the data method of the model which is modified by this patch.



player/lib/models/achievementsmodel.cpp
<http://git.reviewboard.kde.org/r/102476/#comment5389>

    I think I would go for a column enum here (how it is done in the highscoresmodel).
    
    1) It would be more talkative.
    
    2) I would like to make an "extra" enumeration value at the end after this commit. That would eliminate the hard coded number in the column getter. (it is okay since there is no fragmentation in this regard).


- Laszlo


On Aug. 28, 2011, 1:37 p.m., Felix Rohrbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102476/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2011, 1:37 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> Achievements now have icons, AchievementsManager saves the current Score
> 
> 
> Diffs
> -----
> 
>   engine/achievementsmanager.cpp a657074 
>   engine/achievementsmanager.h d2c35a3 
>   engine/achievement.h 84018de 
>   engine/achievement.cpp 75cef86 
>   player/lib/models/achievementsmodel.cpp cb5db2f 
> 
> Diff: http://git.reviewboard.kde.org/r/102476/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Felix
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gluon/attachments/20110828/449738cb/attachment-0001.html>