Review Request: Achievements 12: Add TasksStatistic class

Laszlo Papp lpapp at kde.org
Tue Sep 27 23:40:02 UTC 2011


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



engine/tasksstatistic.h
<http://git.reviewboard.kde.org/r/102700/#comment6101>

    enter to make it consistent :)



engine/tasksstatistic.cpp
<http://git.reviewboard.kde.org/r/102700/#comment6103>

    How about setAccomplishedStatus( index, true ); ?



engine/tasksstatistic.cpp
<http://git.reviewboard.kde.org/r/102700/#comment6104>

    How about setAccomplishedStatus( index, false ); and put the logic into the "container" method instead ?



engine/tasksstatistic.cpp
<http://git.reviewboard.kde.org/r/102700/#comment6102>

    I would personally prefer the implementation here, how the QWidget's setVisible is also implemented. the show() and hide() convenience methods, just call setVisibible, like:
    ...void show() { setVisible(true); }
    and 
    ...void hide() { setVisible(false); }
    
    I think the reason is normally that, the convenience methods are born later, and that wraps the "non-convenient" method up, and not vica versa.Hence in theory, first you need a plain, raw implementation that you can wrap up by using convenient methods. I hope I am not mistaken here. =)


- Laszlo Papp


On Sept. 27, 2011, 4:07 p.m., Felix Rohrbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102700/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2011, 4:07 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Description
> -------
> 
> Have a number of tasks in one statistic that can be accomplished in any order
> 
> 
> Diffs
> -----
> 
>   engine/CMakeLists.txt 6e13e5a 
>   engine/assets/other/statistics/statisticsasset.h f4eaa0f 
>   engine/assets/other/statistics/statisticsasset.cpp 69b74b1 
>   engine/tasksstatistic.h PRE-CREATION 
>   engine/tasksstatistic.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102700/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Felix Rohrbach
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gluon/attachments/20110927/5b9980fa/attachment-0001.html>


More information about the Gluon mailing list