Review Request: Achievements 6: AchievementsManager

[Laszlo Papp]

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



engine/achievementsmanager.h
<http://git.reviewboard.kde.org/r/102034/#comment4330>

    As Dan already started pointing it out. It would also be nice to put something, like:
    
    "index must be a valid index position in the list (i.e., 0 <= i < achievementsCount/count/foobar())." I would personally use .at() inside the methods since that does not cause a deep copy. I think that check is acceptable what has been said (even if the QList and STL lists do not use check, just "segfault" for speed optimization).



engine/achievementsmanager.cpp
<http://git.reviewboard.kde.org/r/102034/#comment4324>

    The "cleanup" meaning is somewhat implied here.
    
    Just briefly in 1-2 few words why (or if you would like to keep it this way, capital C :)



engine/achievementsmanager.cpp
<http://git.reviewboard.kde.org/r/102034/#comment4328>

    "Number of Achievements:" ? 
    
    Note: You do not need to put "space", if you use qDebug(), that is implied.



engine/achievementsmanager.cpp
<http://git.reviewboard.kde.org/r/102034/#comment4327>

    Just briefly in 1-2 few words why (or if you would like to keep it this way, capital C :)



engine/achievementsmanager.cpp
<http://git.reviewboard.kde.org/r/102034/#comment4315>

    Same issue here about the deleteLater. qDeleteLaterAll is not implemented yet. :p



engine/achievementsmanager.cpp
<http://git.reviewboard.kde.org/r/102034/#comment4329>

    You could also make some error/warning log here.


- Laszlo


On July 21, 2011, 5:32 p.m., Felix Rohrbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102034/
> -----------------------------------------------------------
> 
> (Updated July 21, 2011, 5:32 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> This class saves the achievements information into a separate file, so that achievements can be shown without opening the whole project.
> 
> 
> Diffs
> -----
> 
>   engine/CMakeLists.txt 59acf45 
>   engine/achievementsmanager.h PRE-CREATION 
>   engine/achievementsmanager.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102034/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Felix
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/gluon/attachments/20110721/c66d159b/attachment-0001.htm