Review Request 111657: Project performance chart refresh

[Friedrich W. H. Kossebau]

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


The first patch, for the bug, IMHO is better done in another way, so please update it.

And the second patch might result in errors perhaps (no idea really, don't know the code enough yet), so unless that is a real performance killer I would prefer to have that just a TODO comment for now.


plan/libs/kernel/kpttask.cpp
<http://git.reviewboard.kde.org/r/111657/#comment26912>

    Not sure CompletionActualEffort is the proper enum. Sadly they are not really documented, and then they seem to all make no difference anyway.
    
    So instead of reusing that one just add another item to enum Node::Properties, named UsedEffort, and use this as value for the changed(...) call.  And make it also results in clearing all clearPerformanceCaches, like the other enum values in Node::changed(Node *node, int property).



plan/libs/models/kptnodechartmodel.cpp
<http://git.reviewboard.kde.org/r/111657/#comment26911>

    Is there really only the projectNode which is without parents? I am not yet sure if there cannot be other nodes which would meet the condition.
    
    Especially as the whole logic might be there for a reason, so I would be rather cautious to change this.
    
    Perhaps just add a TODO for now, and once you feel really secure that this is the right optimization, do it.


- Friedrich W. H. Kossebau


On July 23, 2013, 1 p.m., Alvaro Soliverez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111657/
> -----------------------------------------------------------
> 
> (Updated July 23, 2013, 1 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> * When effort is added to a task, use the Node::CompletionActualEffort property to have the performance cache should be refreshed when it hits Node::changed() method. 
> This solves bug 322735, where the project performance chart is not refreshed. This is because there is a cache that does not get refreshed unless the changed() method is called with the correct property.
> 
> * Instead of traversing all nodes looking for the one without parent, request the project for that node using projectNode(). Just a small optimization. The one node without parent is the project node, which can be easily requested, instead of making a O(n2) loop.
> 
> 
> This addresses bug 322735.
>     http://bugs.kde.org/show_bug.cgi?id=322735
> 
> 
> Diffs
> -----
> 
>   plan/libs/kernel/kpttask.cpp da48be0 
>   plan/libs/models/kptnodechartmodel.cpp 655dfc8 
> 
> Diff: http://git.reviewboard.kde.org/r/111657/diff/
> 
> 
> Testing
> -------
> 
> Tested manually. The performance chart is refreshed correctly and there were no apparent performance drops or other side-effects.
> 
> 
> Thanks,
> 
> Alvaro Soliverez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130724/87c1fc12/attachment.htm>